fix: fatal php shutdowns

[AlliBalliBaba]

Fixes #2268 (and maybe others)

In that issue, a timeout during a curl_multi request leads to a fatal error and bailout during php_request_shutdown(). After looking at the FPM implementation I realized it also wraps php_request_shutdown() with a zend_bailout, which we don't. This PR wraps the shutdown function and restarts ZTS in case of an unexpected bailout, which fixes #2268 and prevents any potential crashes from bailouts during shutdown.

Still a draft since I think it might make sense to wrap the whole request loop in a zend_try.

[henderkes]

I'm a bit confused. It seems that fpm doesn't specifically wrap the shutdown, but the whole request accept in a zend_first_try. Don't we do that too - and wouldn't the zend_try fail if there weren't an outer zend_first_try?

[AlliBalliBaba]

FPM wraps the whole while loop accepting requests, we don't, we only wrap the actual script execution.

Not sure what else can lead to a bailout. I just know that bailing out without a try-catch will crash the process, like in the muli curl reproducer from #2268.

We could also wrap the entire while loop, might be better for performance.

[henderkes]

Yeah we should wrap the loop then, too and exit out gracefully to restart the thread. Less impact on the good case (no crash).

[AlliBalliBaba]

php_execute_script actually has it's own zend_try for fatal errors, same with shutdown functions. All other bailouts will mainly happen in extensions and probably require a thread restart, which is what this PR now does. This makes it behave more like FPM in worst case scenarios, which restarts the process.

The only unfortunate thing is that I don't know how to test this, maybe with a custom extension that intentionally fails on shutdown?

[henderkes]

The only unfortunate thing is that I don't know how to test this, maybe with a custom extension that intentionally fails on shutdown?

Not a good idea, we'd need to compile it in statically for tests and make it only load on some signal we give it.

Perhaps the max_execution_time could be provoked?

[AlliBalliBaba]

max_execution_time will bail out to a separate try/catch inside of php_execute_script. To provoke a bailout here, something has to go wrong in extension startup/shutdown.

Maybe it would be possible to create a failing extension via extension tests? Hmm maybe I'll find a way to corrupt an extension, otherwise we can also just not explicitly test.