php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #54157 Destruction of some global variables before session write handler callback
Submitted: 2011-03-04 00:54 UTC Modified: 2011-03-17 23:03 UTC
Votes:1
Avg. Score:5.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: tstarling at wikimedia dot org Assigned:
Status: Wont fix Package: Scripting Engine problem
PHP Version: 5.3SVN-2011-03-03 (SVN) OS: Linux
Private report: No CVE-ID: None
 [2011-03-04 00:54 UTC] tstarling at wikimedia dot org
Description:
------------
The request function of the session module calls custom session save handlers. Because modules are shut down after zend_call_destructors() is called, this exposes the weird and apparently broken behaviour of zend_call_destructors() to the user space. 

Its effect is to delete all global variables with a reference count of 1, but only if they hold an object. Global variables which are referenced from anywhere, including other global variables, are not deleted. 

Ideally, I would like session save handlers to have reliable access to global variables. That probably means calling them before the user-defined __destruct() functions, say with an extra hook into php_request_shutdown(). 

Failing that, I would like the behaviour to be consistent and predictable, so that we don't end up with strange regressions like the one observed here:

http://www.mediawiki.org/wiki/Special:Code/MediaWiki/83140#c14601

Test script:
---------------
<?php

function noop() {}
function save() {
	debug_zval_dump( $GLOBALS['a'] );
	debug_zval_dump( $GLOBALS['b'] );
	debug_zval_dump( $GLOBALS['c'] );
}
class Foo {}

$a = new Foo;
$b = new Foo;
$c =& $b;

session_set_save_handler( 'noop', 'noop', 'noop', 'save', 'noop', 'noop' );
session_start();



Expected result:
----------------
object(Foo)#1 (0) refcount(2){
}
object(Foo)#2 (0) refcount(1){
}
object(Foo)#2 (0) refcount(1){
}


Actual result:
--------------
Notice:  Undefined index: a in /home/tstarling/src/php/stuff/weird-shutdown_destructors.php on line 5
NULL refcount(1)
object(Foo)#2 (0) refcount(1){
}
object(Foo)#2 (0) refcount(1){
}


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2011-03-04 01:29 UTC] tstarling at wikimedia dot org
Here's another argument in favour of adding a new hook to php_request_shutdown(): the point of session_set_save_handler() is to allow userspace code to implement alternative methods of session storage. Writing to alternate storage will typically involve calling some other PHP module. But how can we safely call other PHP modules when half of them will have had their RSHUTDOWN functions called? There is the potential for subtle bugs dependent on module load order: dangling pointers, memory corruption, etc.
 [2011-03-04 10:19 UTC] johannes@php.net
This is a hen and egg problem. First issue which comes to mind is: What happens if a destructor wants to log it's state or such in the session?

We have tuned the order multiple times and the current behaviour is the best of the bad choices we have.
 [2011-03-04 10:20 UTC] johannes@php.net
-Status: Open +Status: Wont fix
 [2011-03-17 14:53 UTC] nicolas dot grekas+php at gmail dot com
If you add this to your code, it works around the problem, while still closing the session as late as possible I think :

<?php
register_shutdown_function('shutdown_session');
function shutdown_session() {register_shutdown_function('session_write_close');}
?>
 [2011-03-17 23:03 UTC] tstarling at wikimedia dot org
@nicolas: Yes, I know, I did it before I reported this bug. I linked to the revision in my internals post. 

http://marc.info/?l=php-internals&m=130025049301140&w=2
 [2011-03-18 14:14 UTC] nicolas dot grekas+php at gmail dot com
@tstarling: read your patch. My code above has one more thing you maybe didn't notice: I call register_shutdown_function twice. First to register shutdown_session, then inside shutdown_session to register session_write_close. This way, I push the session_write_close call after other shutdown registered functions.

So I would change your memsess_write_close function definition at http://www.mediawiki.org/wiki/Special:Code/MediaWiki/83147 with :

 function memsess_write_close() {
-	session_write_close();
+	register_shutdown_function('session_write_close');
 }
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Oct 05 00:01:29 2024 UTC