Don't reset the timeout on deactivate when the timeout was changed#6683
Don't reset the timeout on deactivate when the timeout was changed#6683NattyNarwhal wants to merge 1 commit intophp:masterfrom
Conversation
When the time limit for a script is changed, when the script ends, its INI value will be reset. This calls the event handler for the timeout change, which will unset then reset the timeout. However, this is done even if the script is done executing, and say, the CGI or CLI web server process is idle. This is probably incorrect, but isn't a problem on most platforms, because PHP uses a timer that only ticks when the process is active (that is, executing code). Since when it's idle, it's blocking on listen/read, it won't tick because nothing executes. However, on platforms where only the real-time timer is supported, (Cygwin/PASE) it ticks regardless of if PHP is even executing. This means that the idle processes are subject to timeouts from the INI reset on script end. This makes it so the timer is never set if the state is deactivating. Testing with the CLI web server indicates the timer no longer spuriously activates under PASE. Fixes PHP bug #80728
|
cc @kocsismate |
|
@kocsismate I think you tried to deal with this issue in as part of #6504, but in a more complicated way (extra skip_on_update flag?) |
|
Ah, I didn't know wall time across all platforms was something that was being considered. A really simple implementation of that would probably be just collapse everything to the real-time timer instead, if you didn't mind completely changing semantics. As of right now, only i/Cygwin are doing it that way. (Which makes me realize I should probably make a note of that in the docs...) |
|
Poking to see if this patch is still interesting in the wake of the other one. I'm also interested in the backporting policy; curious if this is OK for 7.4/8.0. |
|
Hate to sound like a bother, but bumping. There's a feature freeze soon that might block the other PR, so if that PR gets blocked/declined, this one would at least keep the original behaviour consistent. |
|
The alternative here would be to change the shutdown sequence and move zend_unset_timeout (currently stage 4) after zend_deactivate (currently stage 9). That means the timeout is active for larger parts of the shutdown sequence though, so I think your patch may be preferable as a more conservative approach. |
3 participants
When the time limit for a script is changed, when the script ends, its INI value will be reset. This calls the event handler for the
timeout change, which will unset then reset the timeout. However, this is done even if the script is done executing, and say, the CGI or CLI web server process is idle.
This is probably incorrect, but isn't a problem on most platforms, because PHP uses a timer that only ticks when the process is active (that is, executing code). Since when it's idle, it's blocking on listen/read, it won't tick because nothing executes. However, on platforms where only the real-time timer is supported, (Cygwin/PASE) it ticks regardless of if PHP is even executing. This means that the idle processes are subject to timeouts from the INI reset on script end.
This makes it so the timer is never set if the state is deactivating. Testing with the CLI web server indicates the timer no longer
spuriously activates under PASE.
Fixes PHP bug #80728