Fix ht corruption during shutdown for bailing user stream

[iluuu1994]

Fixes GH-19844

[nielsdos]

This looks hacky and will likely cause leaks for resources that were not yet destroyed.

[iluuu1994]

I'm not sure there are any good solutions for bailout during shutdown. We might minimize leaking by wrapping each resource destructor call in zend_try. I'm open to suggestions.

Also note that it at least does not introduce any new leaks. The current code would already leak if the second attempt to call the destructor bails again.

[arnaud-lb]

I agree this is better than before, and this is similar to how we handle bailouts in shutdown_destructors(). However there is more risk of leaking scarce resources (file descriptors) in this case than with destructors, so it may be worth it to try to release them anyway. zend_close_rsrc_list() could wrap the entire loop in zend_try and restart to the next resource after a bailout.

[iluuu1994]

Okay, I adjusted the code to keep closing resources even after bailout, so that the resources don't leak.

[iluuu1994]

Unfortunately the Windows issue is legit, but I don't know what causes it and I don't have a Windows machine to test it.

[arnaud-lb]

I've tried running the test in a VM, but it passed

[nielsdos]

That termsig code converts to 0xc00000ff in hex. That means STATUS_BAD_FUNCTION_TABLE. Prediction: if you run CI without JIT, issue disappears?
Can't check now as I have no dev machine at hand

[nielsdos]

FWIW I can reproduce the issue with a release build on Windows,~~ even without this PR but with the test in this PR. Doesn't even need JIT/opcache.~~ EDIT: no wait, it does need the patch, this is über confusing.

001- Fatal error: Bail in %s on line %d
002-
003- Fatal error: Bail in %s on line %d
004-
     Fatal error: Bail in C:\php-sdk\phpdev\vs17\x64\php-src\x.php on line 22
002+
003+ Termsig=-1073741784

[nielsdos]

For some reason it works when you don't remove the try-catch in zend_execute_API.c...

[iluuu1994]

So, goto misoptimization or something?

[nielsdos]

So, goto misoptimization or something?

I thought that first too.
It already failed in CI for some reason, even though the while loop works on my machine... I'll try pushing the one where we undo the zend_try in zend_execute_API.c

[nielsdos]

Alternatively if looping / goto is an issue, zend_try around the zend_resource_dtor call itself could also work (but may be more performance hungry).

[iluuu1994]

Seems like neither worked... If this is an existing issue, maybe it's ok to skip the test on Windows?

[nielsdos]

Seems like neither worked... If this is an existing issue, maybe it's ok to skip the test on Windows?

This sucks, because reproducing it is also flaky. Yesterday it seemed like it is pre-existing but now it doesn't reproduce anymore. I'll try to have one more look again under a debugger in the hopes I can figure out why it is failing.

[iluuu1994]

Thanks for your effort! I was just asking because I'm unable to help. :) But if you prefer we can wait until the issue is solved.

[nielsdos]

No dice... It just stops reproducing when trying to debug it. 🤷