Re: [PHP-DEV] Safe timeout handling

[Matt Wilmas]

Hi Bob, all,

- Original Message 
From: "Bob Weinand"
Sent: Wednesday, April 20, 2016



> Am 20.04.2016 um 18:22 schrieb 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.


I looked at it; if we had an already existing if (EG(exception)) branch 
there, we could actually save something, but Dmitry put it just in jumping 
ops, where we never have an exception check. (And there we need a check as 
else a while(1) {} would never be timed out - i.e. when there are no ops 
inside which actually do a check_exception) … so this 
EG(something_unexpected_to_check) is even more expensive.


Yeah, when I was going to reply, I went to look again and saw that 
EG(exception) isn't explicitly checked nearly as often as I thought!


But despite it adding 0.5~1% more instructions, according to Dmitry, one can 
see in the Intel benchmark e-mails that it doesn't really hurt anything (at 
all).  That's expected, really.  The extra instructions, I believe, are 
independent, e.g. the variable can be loaded and tested ahead of time while 
just a few other instructions are doing work.  And the branch is perfectly 
predictable and never taken, which is as good as you can get...


Things would be easy if we could just alter the return addresses of the 
opcode handlers or similar magic, but I doubt that's a very nice 
cross-platform solution...


What were you thinking of?  That wouldn't work anyway with handlers that 
don't return (GOTO VM, etc.).  But, it doesn't really seem to be anything to 
worry about now with the benchmarks.



If you have an even better idea than I do… please show a PoC :-)

Thanks,
Bob


- Matt 



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Safe timeout handling

[Julien Pauli]

On Wed, Apr 20, 2016 at 12:58 PM, Dmitry Stogov  wrote:
> 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.
>
>
> 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.
>
> 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.


Hi !

Good new.

No RFC needed, but that breaks ABI and could impact extensions.
So, 7.1 as target is OK, not 7.0


Julien.P

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

RE: [PHP-DEV] Safe timeout handling

[Anatol Belski]

Hi Dmitry,

> -Original Message-
> From: Dmitry Stogov [mailto:dmi...@zend.com]
> Sent: Wednesday, April 20, 2016 12:58 PM
> To: Nikita Popov <nikita@gmail.com>; Rasmus Lerdorf
> <ras...@lerdorf.com>; Anatol Belski <a...@php.net>; Antony Dovgal
> <antony.dov...@gmail.com>; Zeev Suraski <z...@zend.com>; Xinchen Hui
> <larue...@php.net>
> Cc: internals <internals@lists.php.net>
> Subject: [PHP-DEV] Safe timeout handling
> 
> 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.
> 
> 
> 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.
> 
> 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.
> 
I've tested your patch with CLI, mpm_prefork, mpm_worker and mpm_winnt and
see no regressions. The existing tests in tests/*/*timeout*.phpt pass as
well. We'd probably need to add some more with respect to the new INI option
and more extensive testing under high load.

AFM it's a good step towards unifying the timeout handling and making it
safer. Even better if ticks and exceptions handling can be later improved by
the same approach. 

Regards

Anatol


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Safe timeout handling

[Dmitry Stogov]

On 04/20/2016 08:40 PM, Bob Weinand wrote:


Am 20.04.2016 um 18:22 schrieb 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.


I looked at it; if we had an already existing if (EG(exception)) 
branch there, we could actually save something, but Dmitry put it just 
in jumping ops, where we never have an exception check. (And there we 
need a check as else a while(1) {} would never be timed out - i.e. 
when there are no ops inside which actually do a check_exception) … so 
this EG(something_unexpected_to_check) is even more expensive.
I meant later we may replace check for EG(timed_out) with check for 
EG(something_unexpected_to_check), and then perform additional checks in 
zend_vm_interrupt().


Thanks. Dmitry.



Things would be easy if we could just alter the return addresses of 
the opcode handlers or similar magic, but I doubt that's a very nice 
cross-platform solution...


If you have an even better idea than I do… please show a PoC :-)

Thanks,
Bob


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

Re: [PHP-DEV] Safe timeout handling

[Bob Weinand]

> Am 20.04.2016 um 18:22 schrieb 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.

I looked at it; if we had an already existing if (EG(exception)) branch there, 
we could actually save something, but Dmitry put it just in jumping ops, where 
we never have an exception check. (And there we need a check as else a while(1) 
{} would never be timed out - i.e. when there are no ops inside which actually 
do a check_exception) … so this EG(something_unexpected_to_check) is even more 
expensive.

Things would be easy if we could just alter the return addresses of the opcode 
handlers or similar magic, but I doubt that's a very nice cross-platform 
solution...

If you have an even better idea than I do… please show a PoC :-)

Thanks,
Bob

> 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

Re: [PHP-DEV] Safe timeout handling

[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



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Safe timeout handling

[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 



--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

[PHP-DEV] Safe timeout handling

[Dmitry Stogov]

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.


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.

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.