Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92551
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 68.114.190.30 cause and error)
Message-ID: <59CCDF7AE84348429CD37A2A878B29C8@pc1>
To: "Dmitry Stogov" <dmitry@zend.com>,
	"Bob Weinand" <bobwei9@hotmail.com>,
	"Nikita Popov" <nikita.ppv@gmail.com>,
	"Rasmus Lerdorf" <rasmus@lerdorf.com>,
	"Anatol Belski" <ab@php.net>,
	"Antony Dovgal" <antony.dovgal@gmail.com>,
	"Zeev Suraski" <zeev@zend.com>,
	"Xinchen Hui" <laruence@php.net>
Cc: "internals" <internals@lists.php.net>
References: <BY2PR0201MB1784CF39602D523115E594A6BF6D0@BY2PR0201MB1784.namprd02.prod.outlook.com>
Date: Wed, 20 Apr 2016 10:24:00 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Safe timeout handling
From: php_lists@realplain.com ("Matt Wilmas")

Hi Dmitry,

----- Original Message -----
From: "Dmitry Stogov"
Sent: Wednesday, April 20, 2016

> Hi,
>
>
> It's a well known PHP problem, that exceeding of execution time-out
> (max_execution_time) may lead to unexpected crashes.
>
> They occur because PHP may be interrupted in inconsistent state, and 
> attempt
> to release allocated by request resources leads to failure.
>
> Almost any big site sees these crashes from time to time.
>
>
> I propose to delay actual request termination until a "safe" point in
> interpreter.
>
> Signal handler will just set EG(timed_out) flag.
>
> Interpreter will check this time from time to time (on jumps and calls 
> that
> may make loops or recursion) and perform the actual termination.
>
> This approach already works in PHP for Windows.

I was thinking about this, checking for things like EG(exception) 
"constantly," a few months ago for another reason...

What about instead of adding additional checks in the same place(s) in VM, 
we just limit it to 1 check, for multiple things?  Just have 
EG(something_unexpected_to_check), and behind that (or in a function, I 
guess), the actual rare/unexpected thing gets checked: timed_out, exception, 
etc.

It seems Bob had a similar idea in the PR comment, except literally using 
exceptions...

> In addition I introduce hard_timeout (default value 2 seconds).
>
> In case the "soft" timeout wasn't handled "safely" in that 2 seconds
> (because of long running internal function), PHP process will be 
> terminated
> without attempt to free any resources.
>
> ZTS build will ignore "hard_timeout" (in the same way as PHP on Windows 
> do).
>
>
> The PR: https://github.com/php/php-src/pull/1876
>
>
> It removes "exit_on_timeout" ini directive, and introduces "hard_timeout"
> instead.
>
> Additional checks in VM make 0.5-1% slowdown in term of instruction 
> retired
> reported by callgrind.

A single check would save those additional instructions and branches, and 
would actually improve things on Windows (since this PR doesn't change 
anything there).

> I think we don't need RFC for this. This is a long time desired fix.
>
>
> The same "interrupt" handling mechanism in the future may be reused for 
> TICK
> and signal handling.
>
>
> Thanks. Dmitry.

- Matt