Skip to content

Fix GH-23061: SessionHandler::create_sid() failure leaks memory in debug build - #23064

Open
lazerg wants to merge 2 commits into
php:PHP-8.4from
lazerg:fix/gh-23061-create-sid-leak
Open

Fix GH-23061: SessionHandler::create_sid() failure leaks memory in debug build#23064
lazerg wants to merge 2 commits into
php:PHP-8.4from
lazerg:fix/gh-23061-create-sid-leak

Conversation

@lazerg

@lazerg lazerg commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

When a save handler extending SessionHandler throws while the session is starting, php_session_abort() calls the user close callback, but zend_call_function() refuses to run userland code while an exception is pending. SessionHandler::close() therefore never runs, and the default handler that SessionHandler::open() opened stays open, so its mod_data is never freed. A debug build reports it as two leaks.

Since PS(mod_user_is_open) already tracks whether the default handler is open, request shutdown can close it when the user handler did not. This also covers a handler that overrides close() without calling parent::close(), which leaked the same way.

Fixes GH-23061

Comment thread ext/session/session.c
@lazerg
lazerg requested a review from jorgsowa as a code owner August 7, 2026 11:04
Comment thread ext/session/session.c
Comment on lines +148 to +155
/* The user handler may not have closed the default handler it opened, e.g. because a pending
* exception prevented its close callback from running at all */
if (PS(mod_user_is_open)) {
zend_try {
PS(default_mod)->s_close(&PS(mod_data));
} zend_end_try();
PS(mod_user_is_open) = false;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still needed?

@lazerg lazerg Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, different path. This one guards normal shutdown when a user close() never calls parent::close(). The abort() check only covers session_start() failing outright, it never reaches this function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants