Fix GH-17509: Apache parent and subrequest double bailout

[bukka]

What happens is that zend_first_try sets EG(bailout) to NULL. If that happens in subrequest that shares the globals with parent request, then the EG(bailout) gets reset for the parent and if there is a bailout in the parent, it fails because the jump address is not set. I think that technically we don't really need zend_first_try but I guess it's kind of a protection against extra jump after the last bailout. This restores the parent bailout at the end of the handler so parent knows where to jump.

[bukka]

Just few notes.

It fixes GH-17509.

There is currently no test setup but I just started working on it as part of WST project: https://github.com/wstool/wst-php-apache2handler . There are still some issues that I need to sort out. After I make it work I will also add a test for this. Eventually I will also think how to best integrate it to the PHP CI.

[bukka]

Hmm doesn't look like this is completely fixed if there are more subrequests like in https://bugs.php.net/bug.php?id=80558 . Moving to draft

[bukka]

So I managed to fix the empty bailout target but it just showed how this model of running error handler subrequest is completely wrong. Initially it started segfaulting on the freed CWDG because it basically does it twice request shutdown. Just fixing that shows other segfaults. Even if the error page does not bail out, it results in bunch of memory leaks.

This really need some rethinking as it looks like a proper can of worms...

[bukka]

So after fixing CWDG issue, it just needed to add skip in php_request_shutdown if already in shutdown which made the things work more cleanly except sharing the globals between the parent and subrequest (function redefinition) but don't think we can do much about that. It's also breaking preloading so it will require some tweaks.

It will also need much more testing and I have a feeling there will be potentially more issues. My testing framework (wst) is currently crashing with prefork (works fine with event so something prefork specific) for some reason. It's quite hard to debug it but will need to figure out somehow.

All in all it seems like quite a bit of work to sort this out and get properly tested.

[nielsdos]

I haven't looked in detail, but gut feelings tells me there are many more issues, and this isn't the right approach of dealing with it? If the EG globals are overridden because the RINIT/RSHUTDOWN sequences re-run then a lot of other state can be corrupted as well. And other module globals are likely susceptible as well. We would need to snapshot & restore the global states between parent and subrequest. This seems annoying to support properly.

[bukka]

Yeah there are more issues (see for example #17655 ). This particular problem is that if there are more bailouts, it just fails in an incorrect way. The idea is to prevent doing shutdown twice and persist just the jump addr. But I wouldn't be suprised if this could actually uncover more issues because bailout would work - that's why I mentioned that this would need a lot more testing

The question is whether it makes sense to try to fix this case as a bug in this way. I'm not sure about it already as it's introducing a new EG flag which could technically be considered an ABI break (in case some extension would make use of flags). So I think it might need go to only master anyway.

It might make sense to look into a global persisting fix in any case. I just had a quick look and globals are in the module so theoretical it could just iterate through all modules, store it and run globals ctor. It will need to store it somewhere so it could either extend the module or keep it in local hash table in handler (in a similar way how I'm storing jump addr in this PR). I'm probably missing something but if it could be all handled in handler, then it could even merged as a bug fix. I will try to look into it more and see.