From 8f21237658d7bf7557df617e85d977d24518c902 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 10 Jan 2023 22:13:56 +0100 Subject: [PATCH 1/3] Introduce zend_bailout_without_gc_protect() This allows users to bail out of a try-catch without protecting the GC in case the error is recoverable and normal execution will continue. --- Zend/zend.c | 10 +++++++--- Zend/zend.h | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index 8fea4e57e5e6..9867eca3881b 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1249,14 +1249,12 @@ ZEND_COLD void zenderror(const char *error) /* {{{ */ } /* }}} */ -ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32_t lineno) /* {{{ */ +ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout_without_gc_protect(const char *filename, uint32_t lineno) { - if (!EG(bailout)) { zend_output_debug_string(1, "%s(%d) : Bailed out without a bailout address!", filename, lineno); exit(-1); } - gc_protect(1); CG(unclean_shutdown) = 1; CG(active_class_entry) = NULL; CG(in_compilation) = 0; @@ -1264,6 +1262,12 @@ ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32 EG(current_execute_data) = NULL; LONGJMP(*EG(bailout), FAILURE); } + +ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32_t lineno) /* {{{ */ +{ + gc_protect(true); + _zend_bailout_without_gc_protect(filename, lineno); +} /* }}} */ ZEND_API size_t zend_get_page_size(void) diff --git a/Zend/zend.h b/Zend/zend.h index 94eafdae2124..29e3c009e173 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -265,6 +265,7 @@ typedef struct _zend_utility_values { typedef size_t (*zend_write_func_t)(const char *str, size_t str_length); +#define zend_bailout_without_gc_protect() _zend_bailout_without_gc_protect(__FILE__, __LINE__) #define zend_bailout() _zend_bailout(__FILE__, __LINE__) #define zend_try \ @@ -293,6 +294,7 @@ zend_result zend_post_startup(void); void zend_set_utility_values(zend_utility_values *utility_values); void zend_unload_modules(void); +ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout_without_gc_protect(const char *filename, uint32_t lineno); ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32_t lineno); ZEND_API size_t zend_get_page_size(void); From 126e2d5abeee1524cdd98b18633ab8a0406c5e9e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 10 Jan 2023 22:28:40 +0100 Subject: [PATCH 2/3] Add regression test for GH-10026 Co-authored-by: abienvenu --- ext/soap/tests/gh10026.phpt | 65 +++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 ext/soap/tests/gh10026.phpt diff --git a/ext/soap/tests/gh10026.phpt b/ext/soap/tests/gh10026.phpt new file mode 100644 index 000000000000..cb1627c59440 --- /dev/null +++ b/ext/soap/tests/gh10026.phpt @@ -0,0 +1,65 @@ +--TEST-- +GH-10026 (Garbage collection stops after exception in SoapClient) +--EXTENSIONS-- +soap +--FILE-- +b = $b; + $b->a = $a; +} + +function failySoapCall() +{ + try { + new SoapClient('https://127.0.0.1/?WSDL'); + } catch (Exception) { + echo "Soap call failed\n"; + } +} + +function gc() +{ + gc_collect_cycles(); + echo "GC Runs: " . gc_status()['runs'] . "\n"; +} + + +buildCycle(); +gc(); +buildCycle(); +gc(); +buildCycle(); +gc(); + +failySoapCall(); + +buildCycle(); +gc(); +buildCycle(); +gc(); +buildCycle(); +gc(); +?> +--EXPECT-- +GC Runs: 1 +GC Runs: 2 +GC Runs: 3 +Soap call failed +GC Runs: 4 +GC Runs: 5 +GC Runs: 6 From 9d761842590cb46b66a7fb95edbe1d4e1633749b Mon Sep 17 00:00:00 2001 From: Valery Ivashchanka Date: Thu, 15 Oct 2026 01:58:50 +0000 Subject: [PATCH 3/3] Fix GH-10026: Garbage collection stops after exception in SoapClient zend_bailout() protects the GC because it assumes the request is over. SoapClient instead catches the bailout, turns the fatal error into a SoapFault and carries on, so the GC stays protected for the rest of the process and no new cycle roots are buffered. Only lift the protection where it is known to be safe: errors raised by ext/soap itself through soap_errorN() are marked, and only for those does the client-side error handler bail out without protecting the GC. Engine fatal errors that SoapClient also converts into a SoapFault (e.g. memory exhaustion or timeouts) keep the GC protected as before. The places that catch a bailout only to clean up and rethrow it now use zend_bailout_without_gc_protect(), so that they preserve whatever the original bailout decided instead of re-protecting the GC. Co-Authored-By: Claude Opus 5.5 --- NEWS | 2 ++ ext/soap/php_encoding.c | 2 +- ext/soap/php_packet_soap.c | 2 +- ext/soap/php_sdl.c | 4 +-- ext/soap/php_soap.h | 25 ++++++++++++++--- ext/soap/soap.c | 34 ++++++++++++++++-------- ext/soap/tests/gh10026_engine_fatal.phpt | 27 +++++++++++++++++++ ext/soap/tests/gh10026_protected.phpt | 25 +++++++++++++++++ 8 files changed, 102 insertions(+), 19 deletions(-) create mode 100644 ext/soap/tests/gh10026_engine_fatal.phpt create mode 100644 ext/soap/tests/gh10026_protected.phpt diff --git a/NEWS b/NEWS index 4be7ba3fdb1d..56f8fa7f44e2 100644 --- a/NEWS +++ b/NEWS @@ -69,6 +69,8 @@ PHP NEWS - SOAP: . Fixed use of uninitialized func in do_request() on OOM bailout. (David Carlier) + . Fixed bug GH-10026 (Garbage collection stops after exception in + SoapClient). (ndossche, Valery Ivashchanka) - Standard: . Fixed chown() and lchown() failing to resolve user names in ZTS builds diff --git a/ext/soap/php_encoding.c b/ext/soap/php_encoding.c index b344af77b3f0..a6ca4f2b394c 100644 --- a/ext/soap/php_encoding.c +++ b/ext/soap/php_encoding.c @@ -1582,7 +1582,7 @@ static zval *to_zval_object_ex(zval *ret, encodeTypePtr type, xmlNodePtr data, z master_to_zval(&data, attr->encode, dummy); } zend_catch { xmlFreeNode(dummy); - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); xmlFreeNode(dummy); set_zval_property(ret, attr->name, &data); diff --git a/ext/soap/php_packet_soap.c b/ext/soap/php_packet_soap.c index 7dcd56f34cce..6bfb0bde805a 100644 --- a/ext/soap/php_packet_soap.c +++ b/ext/soap/php_packet_soap.c @@ -36,7 +36,7 @@ static void master_to_zval_with_doc_cleanup(zval *ret, encodePtr encode, xmlNode zval_ptr_dtor(ret); } xmlFreeDoc(doc); - zend_bailout(); + zend_bailout_without_gc_protect(); } } diff --git a/ext/soap/php_sdl.c b/ext/soap/php_sdl.c index e53da38c33f1..766e0ef7ef51 100644 --- a/ext/soap/php_sdl.c +++ b/ext/soap/php_sdl.c @@ -368,7 +368,7 @@ static void load_wsdl_ex(zval *this_ptr, char *struri, sdlCtx *ctx, int include) load_wsdl_ex(this_ptr, (char*)uri, ctx, 1); } zend_catch { xmlFree(uri); - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); xmlFree(uri); } @@ -1148,7 +1148,7 @@ static sdlPtr load_wsdl(zval *this_ptr, char *struri) } zend_catch { /* Avoid persistent memory leak. */ zend_hash_destroy(&ctx.docs); - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); zend_hash_destroy(&ctx.messages); diff --git a/ext/soap/php_soap.h b/ext/soap/php_soap.h index bc84ea27aa31..1ba2d93e86d8 100644 --- a/ext/soap/php_soap.h +++ b/ext/soap/php_soap.h @@ -163,6 +163,7 @@ ZEND_BEGIN_MODULE_GLOBALS(soap) char* error_code; zval error_object; bool use_soap_error_handler; + bool error_is_own; /* error raised by ext/soap itself via soap_errorN() */ char cache_mode; char cache_enabled; char* cache_dir; @@ -201,16 +202,32 @@ extern zend_class_entry* soap_sdl_class_entry; void add_soap_fault(zval *obj, char *fault_code, char *fault_string, char *fault_actor, zval *fault_detail); #define soap_error0(severity, format) \ - php_error(severity, "SOAP-ERROR: " format) + do { \ + SOAP_GLOBAL(error_is_own) = true; \ + php_error(severity, "SOAP-ERROR: " format); \ + SOAP_GLOBAL(error_is_own) = false; \ + } while (0) #define soap_error1(severity, format, param1) \ - php_error(severity, "SOAP-ERROR: " format, param1) + do { \ + SOAP_GLOBAL(error_is_own) = true; \ + php_error(severity, "SOAP-ERROR: " format, param1); \ + SOAP_GLOBAL(error_is_own) = false; \ + } while (0) #define soap_error2(severity, format, param1, param2) \ - php_error(severity, "SOAP-ERROR: " format, param1, param2) + do { \ + SOAP_GLOBAL(error_is_own) = true; \ + php_error(severity, "SOAP-ERROR: " format, param1, param2); \ + SOAP_GLOBAL(error_is_own) = false; \ + } while (0) #define soap_error3(severity, format, param1, param2, param3) \ - php_error(severity, "SOAP-ERROR: " format, param1, param2, param3) + do { \ + SOAP_GLOBAL(error_is_own) = true; \ + php_error(severity, "SOAP-ERROR: " format, param1, param2, param3); \ + SOAP_GLOBAL(error_is_own) = false; \ + } while (0) static zend_always_inline zval *php_soap_deref(zval *zv) { if (UNEXPECTED(Z_TYPE_P(zv) == IS_REFERENCE)) { diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 74e6422d70c6..4911f0549902 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -123,7 +123,7 @@ static void soap_error_handler(int error_num, zend_string *error_filename, uint3 Z_OBJ(SOAP_GLOBAL(error_object)) = _old_error_object;\ SOAP_GLOBAL(soap_version) = _old_soap_version;\ if (_bailout) {\ - zend_bailout();\ + zend_bailout_without_gc_protect();\ } #define FETCH_THIS_SDL(ss) \ @@ -443,6 +443,7 @@ static void php_soap_init_globals(zend_soap_globals *soap_globals) soap_globals->defEncNs = defEncNs; soap_globals->typemap = NULL; soap_globals->use_soap_error_handler = 0; + soap_globals->error_is_own = false; soap_globals->error_code = NULL; ZVAL_OBJ(&soap_globals->error_object, NULL); soap_globals->sdl = NULL; @@ -1008,7 +1009,7 @@ PHP_METHOD(SoapServer, __construct) } zend_catch { xmlCharEncCloseFunc(service->encoding); service->encoding = NULL; - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); if (service->uri == NULL) { if (service->sdl->target_ns) { @@ -1435,7 +1436,7 @@ PHP_METHOD(SoapServer, handle) } zend_catch { /* Avoid leaking persistent memory */ xmlFreeDoc(doc_request); - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); xmlFreeDoc(doc_request); @@ -1858,7 +1859,7 @@ static ZEND_NORETURN void soap_server_fault(char* code, char* string, char *acto } /* }}} */ -static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, zend_string *error_filename, const uint32_t error_lineno, zend_string *message) /* {{{ */ +static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, zend_string *error_filename, const uint32_t error_lineno, zend_string *message, bool error_is_own) /* {{{ */ { bool _old_in_compilation; zend_execute_data *_old_current_execute_data; @@ -1884,6 +1885,12 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, z add_soap_fault_ex(&fault, &SOAP_GLOBAL(error_object), code, ZSTR_VAL(message), NULL, NULL); Z_ADDREF(fault); zend_throw_exception_object(&fault); + if (error_is_own) { + /* ext/soap raised this error itself at a known point, so execution + * can safely continue after the fault is caught. Engine fatal errors + * (e.g. memory exhaustion, timeouts) keep the GC protected. */ + zend_bailout_without_gc_protect(); + } zend_bailout(); } else if (!use_exceptions || !SOAP_GLOBAL(error_code) || @@ -1954,10 +1961,15 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, z static void soap_error_handler(int error_num, zend_string *error_filename, const uint32_t error_lineno, zend_string *message) /* {{{ */ { + bool error_is_own = SOAP_GLOBAL(error_is_own); + + /* Consume the flag right away: any error raised while handling this one + * (e.g. memory exhaustion) is not a SOAP error. */ + SOAP_GLOBAL(error_is_own) = false; if (EXPECTED(!SOAP_GLOBAL(use_soap_error_handler))) { old_error_handler(error_num, error_filename, error_lineno, message); } else { - soap_real_error_handler(error_num, error_filename, error_lineno, message); + soap_real_error_handler(error_num, error_filename, error_lineno, message, error_is_own); } } /* }}} */ @@ -2286,7 +2298,7 @@ static bool do_request(zval *this_ptr, xmlDoc *request, const char *location, co zval_ptr_dtor(¶ms[0]); xmlFree(buf); if (_bailout) { - zend_bailout(); + zend_bailout_without_gc_protect(); } if (ret && Z_TYPE_P(Z_CLIENT_SOAP_FAULT_P(this_ptr)) == IS_OBJECT) { ret = false; @@ -2429,7 +2441,7 @@ static void do_soap_call(zend_execute_data *execute_data, encode_finish(); if (parse_bailout) { zval_ptr_dtor(&response); - zend_bailout(); + zend_bailout_without_gc_protect(); } } @@ -2483,7 +2495,7 @@ static void do_soap_call(zend_execute_data *execute_data, encode_finish(); if (parse_bailout) { zval_ptr_dtor(&response); - zend_bailout(); + zend_bailout_without_gc_protect(); } } @@ -2531,7 +2543,7 @@ static void do_soap_call(zend_execute_data *execute_data, if (request) { xmlFreeDoc(request); } - zend_bailout(); + zend_bailout_without_gc_protect(); } SOAP_CLIENT_END_CODE(); } @@ -3907,7 +3919,7 @@ static xmlDocPtr serialize_response_call(sdlFunctionPtr function, const char *fu } zend_catch { /* Avoid persistent memory leak. */ xmlFreeDoc(doc); - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); if (function && function->responseName == NULL && @@ -4113,7 +4125,7 @@ static xmlDocPtr serialize_function_call(zval *this_ptr, sdlFunctionPtr function } zend_catch { /* Avoid persistent memory leak. */ xmlFreeDoc(doc); - zend_bailout(); + zend_bailout_without_gc_protect(); } zend_end_try(); return doc; diff --git a/ext/soap/tests/gh10026_engine_fatal.phpt b/ext/soap/tests/gh10026_engine_fatal.phpt new file mode 100644 index 000000000000..37ebaee4aabb --- /dev/null +++ b/ext/soap/tests/gh10026_engine_fatal.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-10026 (Engine fatal errors converted to SoapFault keep the GC protected) +--EXTENSIONS-- +soap +--INI-- +memory_limit=8M +--FILE-- + "test://", "uri" => "urn:test"]); +try { + $client->test(); +} catch (SoapFault $e) { + echo $e->getMessage(), "\n"; +} +var_dump(gc_status()["protected"]); +?> +--EXPECTF-- +bool(false) +Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) +bool(true) diff --git a/ext/soap/tests/gh10026_protected.phpt b/ext/soap/tests/gh10026_protected.phpt new file mode 100644 index 000000000000..099f4ae77727 --- /dev/null +++ b/ext/soap/tests/gh10026_protected.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-10026 (WSDL errors raised by ext/soap must not leave the GC protected) +--EXTENSIONS-- +soap +--FILE-- + WSDL_CACHE_NONE]); +} catch (SoapFault $e) { + echo trim($e->getMessage()), "\n"; +} +var_dump(gc_status()["protected"]); +?> +--CLEAN-- + +--EXPECTF-- +bool(false) +SOAP-ERROR: Parsing WSDL: Couldn't load from %s +bool(false)