Running out of memory in custom session handler's `write` function causes "Cannot call session save handler in a recursive manner"

[ssigwart]

Description

The following code:

<?php

ini_set('memory_limit', '16M');

class TestSessionHandler implements SessionHandlerInterface
{
	public function close(): bool
	{
		error_log("Close");
		return true;
	}
	public function destroy(string $id): bool
	{
		error_log("Destroy");
		return true;
	}
	public function gc(int $max_lifetime): int|false
	{
		error_log("GC");
		return 0;
	}
	public function open(string $path, string $name): bool
	{
		error_log("Open");
		return true;
	}
	public function read(string $id): string|false
	{
		error_log("Read");
		return 'a|i:1234;';
	}
	public function write($id, $data): bool
	{
		error_log("About to run out of memory");
		var_dump(strlen(str_repeat('0', 16777216)));
		error_log("Wrote session");
		return true;
	}
}

session_set_save_handler(new TestSessionHandler());
session_start();
error_log("End of code");

Resulted in this output:

Open
Read
End of code
About to run out of memory
PHP Fatal error:  Allowed memory size of 16777216 bytes exhausted (tried to allocate 16777248 bytes) in /Users/ssigwart/Downloads/sessionsTest.php on line 35

Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 16777248 bytes) in /Users/ssigwart/Downloads/sessionsTest.php on line 35
PHP Warning:  PHP Request Shutdown: Cannot call session save handler in a recursive manner in Unknown on line 0

Warning: PHP Request Shutdown: Cannot call session save handler in a recursive manner in Unknown on line 0
PHP Warning:  PHP Request Shutdown: Failed to write session data using user defined save handler. (session.save_path: /usr/local/share/phpsess, handler: TestSessionHandler::updateTimestamp) in Unknown on line 0

Warning: PHP Request Shutdown: Failed to write session data using user defined save handler. (session.save_path: /usr/local/share/phpsess, handler: TestSessionHandler::updateTimestamp) in Unknown on line 0
Close

But I expected this output instead:

Open
Read
End of code
About to run out of memory
PHP Fatal error:  Allowed memory size of 16777216 bytes exhausted (tried to allocate 16777248 bytes) in /Users/ssigwart/Downloads/sessionsTest.php on line 35

I'd expect the fatal error, but not the recursive session call.

The above simulates the error using a large string, but this happens when a session is large and the custom error handler needs to allocate some more data to write the data.

PHP Version

PHP 8.1.29, PHP 8.3.10

Operating System

No response

[cmb69]

At the end of the script the session is supposed to be written (php_session_save_current_state()), where the OOM error happens. This causes the engine to bailout, so php_session_save_current_state() is called again, and that triggers the "Cannot call session save handler in a recursive manner" warning. Since the bailout error sets CG(unclean_shutdown), we could catch that in the session module, e.g.

 ext/session/mod_user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c
index 168c5c7f1d..b78ef41cad 100644
--- a/ext/session/mod_user.c
+++ b/ext/session/mod_user.c
@@ -26,6 +26,10 @@ const ps_module ps_mod_user = {
 static void ps_call_handler(zval *func, int argc, zval *argv, zval *retval)
 {
 	int i;
+	if (UNEXPECTED(CG(unclean_shutdown))) {
+		ZVAL_UNDEF(retval);
+		return;
+	}
 	if (PS(in_save_handler)) {
 		PS(in_save_handler) = 0;
 		ZVAL_UNDEF(retval);

This looks contrieved, though.

@Girgias, thoughts?

[Girgias]

I'm surprised that the engine will attempt to write to the session in case of a bailout in the first place, as AFAIK destructors are not run.

[cmb69]

php_request_shutdown() first calls the registered shutdown functions (which causes the first call to php_session_save_current_state()), and a bit later zend_deactivate_modules() (which causes the second call to php_session_save_current_state()).

With session_set_save_handler(new TestSessionHandler(), false); I see a somewhat different code path, but still have the same user visible behavior.

[cmb69]

Ah, when also implementing SessionUpdateTimestampHandlerInterface (the name is way too short, by the way ;), and also actually changing $_SESSION, the script behaves as expected by @ssigwart. So part of the problem is PS_UPDATE_TIMESTAMP_FUNC(user) where we "maintain backwards compatibility" and write instead of of updating the timestamp only when not implementing that interface with the short name.