Fix memory_leak in zend_test

Properly use globals init/shutdown to allocate the observer_observe_function_names hashtable instead of attempting to do everything in the ini changed handler

Author: bwoebi

ext/zend_test/observer.c

@@ -184,7 +184,7 @@ static zend_observer_fcall_handlers observer_fcall_init(zend_execute_data *execu
 	} else if (fbc->common.function_name) {
 		if (ZT_G(observer_observe_functions)) {
 			return (zend_observer_fcall_handlers){observer_begin, observer_end};
-		} else if (ZT_G(observer_observe_function_names) && zend_hash_exists(ZT_G(observer_observe_function_names), fbc->common.function_name)) {
+		} else if (zend_hash_exists(ZT_G(observer_observe_function_names), fbc->common.function_name)) {
 			return (zend_observer_fcall_handlers){observer_begin, observer_end};
 		}
 	} else {
@@ -259,40 +259,25 @@ static void fiber_suspend_observer(zend_fiber_context *from, zend_fiber_context
 static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList)
 {
 	zend_array **p = (zend_array **) ZEND_INI_GET_ADDR();
-	if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_SHUTDOWN && (ZT_G(observer_observe_all) || !*p)) {
-		return FAILURE;
-	}
-
 	zend_string *funcname;
 	zend_function *func;
-	if (*p) {
-		if (EG(function_table)) {
-			ZEND_HASH_FOREACH_STR_KEY(*p, funcname) {
-				if ((func = zend_hash_find_ptr(EG(function_table), funcname))) {
-					zend_observer_remove_begin_handler(&func->op_array, observer_begin);
-					zend_observer_remove_end_handler(&func->op_array, observer_end);
-				}
-			} ZEND_HASH_FOREACH_END();
-		}
-		if (stage == PHP_INI_STAGE_STARTUP || stage == PHP_INI_STAGE_SHUTDOWN) {
-			zend_hash_release(*p);
-			*p = NULL;
-		} else {
-			zend_hash_clean(*p);
-		}
+	if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) {
+		ZEND_HASH_FOREACH_STR_KEY(*p, funcname) {
+			if ((func = zend_hash_find_ptr(EG(function_table), funcname))) {
+				zend_observer_remove_begin_handler(&func->op_array, observer_begin);
+				zend_observer_remove_end_handler(&func->op_array, observer_end);
+			}
+		} ZEND_HASH_FOREACH_END();
 	}
+	zend_hash_clean(*p);
 	if (new_value && ZSTR_LEN(new_value)) {
-		if (!*p) {
-			*p = malloc(sizeof(HashTable));
-		}
-		_zend_hash_init(*p, 8, ZVAL_PTR_DTOR, 1);
 		const char *start = ZSTR_VAL(new_value), *ptr;
 		while ((ptr = strchr(start, ','))) {
 			zend_hash_str_add_empty_element(*p, start, ptr - start);
 			start = ptr + 1;
 		}
 		zend_hash_str_add_empty_element(*p, start, ZSTR_VAL(new_value) + ZSTR_LEN(new_value) - start);
-		if (EG(function_table)) {
+		if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) {
 			ZEND_HASH_FOREACH_STR_KEY(*p, funcname) {
 				if ((func = zend_hash_find_ptr(EG(function_table), funcname))) {
 					zend_observer_add_begin_handler(&func->op_array, observer_begin);
@@ -351,8 +336,13 @@ void zend_test_observer_shutdown(SHUTDOWN_FUNC_ARGS)
 	if (type != MODULE_TEMPORARY) {
 		UNREGISTER_INI_ENTRIES();
 	}
+}
 
-	if (ZT_G(observer_observe_function_names)) {
-		zend_hash_release(ZT_G(observer_observe_function_names));
-	}
+void zend_test_observer_ginit(zend_zend_test_globals *zend_test_globals) {
+	zend_test_globals->observer_observe_function_names = malloc(sizeof(HashTable));
+	_zend_hash_init(zend_test_globals->observer_observe_function_names, 8, ZVAL_PTR_DTOR, 1);
+}
+
+void zend_test_observer_gshutdown(zend_zend_test_globals *zend_test_globals) {
+	zend_hash_release(zend_test_globals->observer_observe_function_names);
 }

ext/zend_test/observer.h

@@ -19,5 +19,7 @@
 
 void zend_test_observer_init(INIT_FUNC_ARGS);
 void zend_test_observer_shutdown(SHUTDOWN_FUNC_ARGS);
+void zend_test_observer_ginit(zend_zend_test_globals *zend_test_globals);
+void zend_test_observer_gshutdown(zend_zend_test_globals *zend_test_globals);
 
 #endif

ext/zend_test/test.c

@@ -775,6 +775,13 @@ static PHP_GINIT_FUNCTION(zend_test)
 	ZEND_TSRMLS_CACHE_UPDATE();
 #endif
 	memset(zend_test_globals, 0, sizeof(*zend_test_globals));
+
+	zend_test_observer_ginit(zend_test_globals);
+}
+
+static PHP_GSHUTDOWN_FUNCTION(zend_test)
+{
+	zend_test_observer_gshutdown(zend_test_globals);
 }
 
 PHP_MINFO_FUNCTION(zend_test)
@@ -798,7 +805,7 @@ zend_module_entry zend_test_module_entry = {
 	PHP_ZEND_TEST_VERSION,
 	PHP_MODULE_GLOBALS(zend_test),
 	PHP_GINIT(zend_test),
-	NULL,
+	PHP_GSHUTDOWN(zend_test),
 	NULL,
 	STANDARD_MODULE_PROPERTIES_EX
 };