ext/session: Fix memory leak

[arshidkv12]

Fix the memory leak.

class MySession extends SessionHandler {
    public function validateId($key) {
        return null; //memory leak here
    }
}

$handler = new MySession();

try {
    session_set_save_handler($handler);
    session_start();
} catch (TypeError $e) {
   echo $e->getMessage(), "\n";
}

session_write_close();

try {
    session_start();
} catch (Throwable $e) {
   echo $e->getMessage(), "\n";
}

[devnexen]

what memory leak ? you should probably show its output.

[arshidkv12]

Output:

sapi/cli/php z.php                                              

Fatal error: Uncaught TypeError: Session callback must have a return value of type bool, null returned in /Users/arshid/Downloads/php-src/z.php:18
Stack trace:
#0 /Users/arshid/Downloads/php-src/z.php(18): session_start()
#1 {main}

Next Error: Session id must be a string in /Users/arshid/Downloads/php-src/z.php:18
Stack trace:
#0 /Users/arshid/Downloads/php-src/z.php(18): session_start()
#1 {main}
  thrown in /Users/arshid/Downloads/php-src/z.php on line 18
[Wed Feb 11 19:18:15 2026]  Script:  '/Users/arshid/Downloads/php-src/z.php'
/Users/arshid/Downloads/php-src/Zend/zend_string.h(167) :  Freeing 0x000000010168b230 (80 bytes), script=/Users/arshid/Downloads/php-src/z.php
[Wed Feb 11 19:18:15 2026]  Script:  '/Users/arshid/Downloads/php-src/z.php'
/Users/arshid/Downloads/php-src/ext/session/mod_files.c(415) :  Freeing 0x0000000101690690 (40 bytes), script=/Users/arshid/Downloads/php-src/z.php
=== Total 2 memory leaks detected ===

[devnexen]

that seems a workaround, and probably Gina would prefer to keep as an exception but will leave it to her.

[Girgias]

Yes, just add zval_ptr_dtor(&value); also this is a bug fix and needs to be backported.

[arshidkv12]

Ok. Thank you

[arshidkv12]

Please note: Memory leak is in the PS_OPEN_FUNC(files) function.

	if (!EG(exception)) {
		zend_type_error("Session callback must have a return value of type bool, %s returned", zend_zval_value_name(value)); 
		zval_ptr_dtor(&value);
    }

Output:

sapi/cli/php z.php                                                     
Assertion failed: (zval_gc_type((ref)->gc.u.type_info) == 7 || zval_gc_type((ref)->gc.u.type_info) == 8), function gc_possible_root, file zend_gc.c, line 758.
[36]    60168 abort      sapi/cli/php z.php

[devnexen]

@Girgias seeing Zend/zend_string.h(167) : Freeing 0x000000010168b230 (80 bytes) could it be like more the id global ? if you do not have time I may look at it later.

[devnexen]

I cannot reproduce but can you try the following instead ?

if (!EG(exception)) {
    if (PS(id)) {
        zend_string_release(PS(id));
        PS(id) = NULL;
    }
    zend_type_error("Session callback must have a return value of type bool, %s returned", zend_zval_value_name(value));
}

[devnexen]

I tried with these flags -d session.save_handler=files -d session.save_path=/tmp but could not reproduce

[arshidkv12]

php.ini:

session.use_strict_mode=1
session.name=PHPSESSID
session.save_handler=files

[devnexen]

came up with this draft, do not think it is fully fix all cases tough. @Girgias thoughts ? at least the leak went away.

diff --git a/ext/session/session.c b/ext/session/session.c
index d9a4e2b5a4d..14ce69769f0 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -1725,14 +1725,22 @@ PHPAPI php_session_status php_get_session_status(void)
 
 static bool php_session_abort(void)
 {
-       if (PS(session_status) == php_session_active) {
-               if (PS(mod_data) || PS(mod_user_implemented)) {
-                       PS(mod)->s_close(&PS(mod_data));
-               }
-               PS(session_status) = php_session_none;
-               return true;
-       }
-       return false;
+       if (PS(session_status) == php_session_active) {
+           if (PS(mod_data) || PS(mod_user_implemented)) {
+               zend_object *exception = EG(exception);
+               EG(exception) = NULL;
+               PS(mod)->s_close(&PS(mod_data));
+              // if s_close callback set an exception, we set
+              // it back to the one raised earlier
+              // FIXME: would zend_exception_set_previous be better ?
+               if (exception) {
+                   EG(exception) = exception;
+               }
+           }
+           PS(session_status) = php_session_none;
+           return true;
+       }
+       return false;
 }

[devnexen]

with your case @arshidkv12 I get this after fix

sapi/cli/php a.php 
Session id must be a string