Skip to content

Commit d869ead

Browse files
committed
Fix ZTS zend signal crashes due to NULL globals
Fixes GH-8789. Fixes GH-10015. This is one small part of the underlying bug for GH-10737, as in my attempts to reproduce the issue I constantly hit this crash easily. (The fix for the other underlying issue for that bug will follow soon.) It's possible that a signal arrives at a thread that never handled a PHP request before. This causes the signal globals to dereference a NULL pointer because the TSRM pointers for the thread aren't set up to point to the thread resources yet. PR GH-9766 previously fixed this for master by ignoring the signal if the thread didn't handle a PHP request yet. While this fixes the crash bug, I think the solution is suboptimal for 3 reasons: 1) The signal is ignored and a message is printed saying there is a bug. However, this is not a bug at all. For example in Apache, the signal set up happens on child process creation, and the thread resource creation happens lazily when the first request is handled by the thread. Hence, the fact that the thread resources aren't set up yet is not actually buggy behaviour. 2) I believe since it was believed to be buggy behaviour, that fix was only applied to master, so 8.1 & 8.2 keep on crashing. 3) We can do better than ignoring the signal. By just initialising the resources if they don't exist, in the very same way the request handler in Apache works, we can gracefully handle the signal.
1 parent c4c8d6c commit d869ead

File tree

1 file changed

+7
-0
lines changed

1 file changed

+7
-0
lines changed

Zend/zend_signal.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ void zend_signal_handler_defer(int signo, siginfo_t *siginfo, void *context)
9191
zend_signal_handler(signo, siginfo, context);
9292
return;
9393
}
94+
95+
/* Fetch the thread resource. In case the resource didn't exist yet it will be created.
96+
* This would only be the case if this thread didn't handle a PHP request yet, but
97+
* that's okay as this thread will eventually serve a request anyway.
98+
* This follows the same pattern as e.g. Apache SAPI where the thread resources
99+
* are created lazily. */
100+
(void)ts_resource(0);
94101
#endif
95102

96103
if (EXPECTED(SIGG(active))) {

0 commit comments

Comments
 (0)