Fix GH-10026: Garbage collection stops after exception in SoapClient

[likemusic]

Fixes GH-10026. Supersedes #10283 and keeps its first two commits by @ndossche (the API and the regression test).

Problem

zend_bailout() calls gc_protect(1) on the assumption that the request is ending. SoapClient catches the bailout, turns the fatal error into a SoapFault and carries on, so the GC stays protected for the rest of the process. While protected, gc_possible_root() does not buffer roots, so cycles created from then on are never collected. A single failed WSDL parse (e.g. a malformed local file) is enough, and nothing in userland can undo it.

Why not #10283 as is

As @ndossche pointed out in #10026 (comment), fatal errors also go through this path and need the protection. In soap_real_error_handler() the client converts any E_FATAL_ERRORS into a SoapFault, including memory exhaustion and timeouts. #10283 switches that site to the non-protecting bailout, so an OOM inside a SOAP call would also leave the GC unprotected. gh10026_engine_fatal.phpt covers exactly that case.

Approach

The decision is made where the error is raised, not where it is caught:

• Errors that ext/soap raises itself through soap_errorN() are marked (SOAP_GLOBAL(error_is_own)). soap_error_handler() consumes the flag immediately, so an error raised while handling one (e.g. OOM while building the fault) is not treated as a SOAP error.
• Only for marked errors does the client-side handler bail out with zend_bailout_without_gc_protect(). Engine fatal errors still use zend_bailout() and stay protected. An unmarked SOAP error behaves exactly as today, so the change can only narrow the problem, not widen it.
• The ext/soap sites that catch a bailout only to free memory and rethrow it now use zend_bailout_without_gc_protect(). They preserve the original decision instead of re-protecting the GC (e.g. load_wsdl()'s rethrow would otherwise re-protect after a WSDL error).
• The SoapServer originating sites are unchanged, because the server always ends the request.
• Unlike Fix GH-10026: Garbage collection stops after exception in SoapClient #10283, phar, oci8, pdo_oci, phpdbg and the fuzzer are not touched.

Tests

• gh10026.phpt (from Fix GH-10026: Garbage collection stops after exception in SoapClient #10283): GC runs continue after a failed new SoapClient().
• gh10026_protected.phpt: a malformed local WSDL leaves gc_status()['protected'] false. No network involved.
• gh10026_engine_fatal.phpt: a memory_limit fatal inside __doRequest() still leaves the GC protected.

Without the ext/soap commit, the first two fail and the third passes. With it, all three pass. ext/soap and Zend/tests show no regressions on a debug NTS build.

Scope and open points

• This is a targeted fix for one instance of a general class: code that catches a bailout and continues inherits gc_protected. The general fix (saving gc_protected in zend_try and restoring it in "safe" zend_catch blocks, as @dstogov suggested in Fix GH-10026: Garbage collection stops after exception in SoapClient #10283) needs an audit of every bailout catcher, so it is left out here.
• Naming: I kept zend_bailout_without_gc_protect() from Fix GH-10026: Garbage collection stops after exception in SoapClient #10283. @dstogov preferred zend_bailout_safe()/zend_bailout_clean(), and I'm happy to rename.
• CG(unclean_shutdown) is still set by both variants, as noted by @dstogov, and is left unchanged here.
• Targets PHP-8.4. It is a bug fix with no BC impact on userland.

🤖 Generated with Claude Code

[Girgias]

I'm very much not convinced by this hack.

The correct solution is to refactor ext/soap to throw exceptions rather than use the bailout mechanism. And this would be a master only fix.

[devnexen]

that s also something to definitely avoid in a stable branch.

[devnexen]

this sort of solution around a global is really fragile.

[devnexen]

So I second Girgias here, required changes in master, thanks for the work nonetheless !

[likemusic]

Thanks for the review, @Girgias and @devnexen. Agreed: a new bailout variant in a stable branch and a global flag were the wrong direction.

I opened #24366 against master. The WSDL and XML schema loaders now throw the SoapFault and return instead of bailing out (all 135 soap_errorN() calls in php_sdl.c and php_schema.c), so SoapClient no longer goes through zend_bailout() for these errors and the cycle collector stays enabled. The messages and fault codes are unchanged; SoapServer and exceptions => false still end with the same fatal error. The PR description has the details and how it was tested.

Encoding and decoding errors in php_encoding.c still bail out. That is the next step, as a follow-up PR unless you prefer it in #24366.

Closing this one in favour of #24366.

[ndossche]

But even using exceptions is not that simple: the bailout mechanism is there because fatals need to be caught and converted to a soap fault