Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92557
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 207.46.100.121 as permitted sender)
To: Matt Wilmas <php_lists@realplain.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>
References: <BY2PR0201MB1784CF39602D523115E594A6BF6D0@BY2PR0201MB1784.namprd02.prod.outlook.com>
 <59CCDF7AE84348429CD37A2A878B29C8@pc1>
CC: internals <internals@lists.php.net>
Message-ID: <5717ACAF.8000703@zend.com>
Date: Wed, 20 Apr 2016 19:22:07 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.1
MIME-Version: 1.0
In-Reply-To: <59CCDF7AE84348429CD37A2A878B29C8@pc1>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:23
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2016 16:22:19.4956 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR0201MB1791
Subject: Re: [PHP-DEV] Safe timeout handling
From: dmitry@zend.com (Dmitry Stogov)



On 04/20/2016 06:24 PM, Matt Wilmas wrote:
> 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...

This is a bit different problem. We can't delay EG(exception) checks. I 
thought about a better way of exception handing but didn't find anything 
usable and portable.
>
> 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.

Yes, I have the same idea in background. I even wrote: The same 
"interrupt" handling mechanism in the future may be reused for TICK
and signal handling.

>
> 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).

If you or Bob show me a better working solution (or just PoC), I'll be 
only happy with this.

Thanks. Dmitry.
>
>
>> 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