Shutdown already started modules when a later module init fails

Zend/zend_API.c

@@ -3405,17 +3405,32 @@ void module_registry_unload(const zend_module_entry *module)
 
 ZEND_API void zend_activate_modules(void) /* {{{ */
 {
-	zend_module_entry **p = module_request_startup_handlers;
+	zend_module_entry ** volatile p = module_request_startup_handlers;
 
-	while (*p) {
-		zend_module_entry *module = *p;
+	zend_try {
+		while (*p) {
+			zend_module_entry *module = *p;
 
-		if (module->request_startup_func(module->type, module->module_number)==FAILURE) {
-			zend_error(E_WARNING, "request_startup() for %s module failed", module->name);
-			exit(1);
+			if (module->request_startup_func(module->type, module->module_number)==FAILURE) {
+				zend_error(E_WARNING, "request_startup() for %s module failed", module->name);
+				exit(1);
+			}
+			p++;
 		}
-		p++;
-	}
+	} zend_catch {
+		/* The request shutdown skips RSHUTDOWN unless every RINIT returned,
+		 * release what the modules activated so far hold (e.g. session locks) */
+		while (p > module_request_startup_handlers) {
+			zend_module_entry *module = *--p;
+
+			if (module->request_shutdown_func) {
+				zend_try {
+					module->request_shutdown_func(module->type, module->module_number);
+				} zend_end_try();
+			}
+		}
+		zend_bailout();
+	} zend_end_try();
 }
 /* }}} */
 

ext/session/tests/rinit_bailout_releases_lock.phpt

@@ -0,0 +1,63 @@
+--TEST--
+A request whose startup bails out after session.auto_start releases the session lock
+--EXTENSIONS--
+session
+zend_test
+--SKIPIF--
+<?php
+include 'skipif.inc';
+if (!file_exists(__DIR__ . "/../../../sapi/cli/tests/php_cli_server.inc")) {
+    echo "skip sapi/cli/tests/php_cli_server.inc required but not found";
+}
+?>
+--FILE--
+<?php
+include __DIR__ . "/../../../sapi/cli/tests/php_cli_server.inc";
+
+$savePath = __DIR__ . '/rinit_bailout_releases_lock_sessions';
+@mkdir($savePath);
+
+$args = [
+    '-d', 'extension_dir=' . ini_get('extension_dir'),
+    '-d', 'session.auto_start=1',
+    '-d', 'session.save_handler=files',
+    '-d', 'session.save_path=' . $savePath,
+    '-d', 'zend_test.bailout_in_rinit=1',
+];
+$builtin = shell_exec(escapeshellarg(getenv('TEST_PHP_EXECUTABLE') ?: PHP_BINARY) . ' -n -m');
+foreach (['session', 'zend_test'] as $ext) {
+    if (!preg_match("/^$ext$/m", $builtin)) {
+        array_push($args, '-d', "extension=$ext");
+    }
+}
+
+php_cli_server_start('echo session_id();', null, $args);
+
+function request(): string {
+    $fp = stream_socket_client('tcp://' . PHP_CLI_SERVER_ADDRESS, $errno, $errstr, 5);
+    stream_set_timeout($fp, 5);
+    fwrite($fp, "GET / HTTP/1.0\r\nHost: " . PHP_CLI_SERVER_HOSTNAME . "\r\nCookie: PHPSESSID=rinitbailout\r\n\r\n");
+    $response = stream_get_contents($fp);
+    fclose($fp);
+
+    [, $body] = explode("\r\n\r\n", $response, 2) + ['', ''];
+
+    return "[$body]";
+}
+
+// the first request fails in zend_test's RINIT, after the session module locked the session
+echo request(), "\n";
+// blocks if the session is still locked
+echo request(), "\n";
+?>
+--CLEAN--
+<?php
+$savePath = __DIR__ . '/rinit_bailout_releases_lock_sessions';
+foreach (glob("$savePath/*") as $file) {
+    unlink($file);
+}
+@rmdir($savePath);
+?>
+--EXPECT--
+[]
+[rinitbailout]

ext/zend_test/php_test.h

@@ -58,6 +58,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
 	int register_passes;
 	bool rearm_vm_interrupt;
 	bool print_stderr_mshutdown;
+	zend_long bailout_in_rinit;
 	zend_long limit_copy_file_range;
 	int observe_opline_in_zendmm;
 	zend_mm_heap* zend_orig_heap;

ext/zend_test/test.c

@@ -1374,6 +1374,7 @@ PHP_INI_BEGIN()
 	STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals)
 	STD_PHP_INI_BOOLEAN("zend_test.rearm_vm_interrupt", "0", PHP_INI_SYSTEM, OnUpdateBool, rearm_vm_interrupt, zend_zend_test_globals, zend_test_globals)
 	STD_PHP_INI_BOOLEAN("zend_test.print_stderr_mshutdown", "0", PHP_INI_SYSTEM, OnUpdateBool, print_stderr_mshutdown, zend_zend_test_globals, zend_test_globals)
+	STD_PHP_INI_ENTRY("zend_test.bailout_in_rinit", "0", PHP_INI_SYSTEM, OnUpdateLong, bailout_in_rinit, zend_zend_test_globals, zend_test_globals)
 #ifdef HAVE_COPY_FILE_RANGE
 	STD_PHP_INI_ENTRY("zend_test.limit_copy_file_range", "-1", PHP_INI_ALL, OnUpdateLong, limit_copy_file_range, zend_zend_test_globals, zend_test_globals)
 #endif
@@ -1625,6 +1626,12 @@ PHP_MSHUTDOWN_FUNCTION(zend_test)
 
 PHP_RINIT_FUNCTION(zend_test)
 {
+	/* number of requests whose startup fails */
+	if (ZT_G(bailout_in_rinit) > 0) {
+		ZT_G(bailout_in_rinit)--;
+		zend_bailout();
+	}
+
 	ALLOC_HASHTABLE(ZT_G(global_weakmap));
 	zend_hash_init(ZT_G(global_weakmap), 8, NULL, ZVAL_PTR_DTOR, 0);
 	ZT_G(observer_nesting_depth) = 0;