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/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); 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.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 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)