Hello, [CC to pecl-dev since this is APC-related.]
I have been investigating a problem occuring when using APC in combination with userspace session save handlers. This has been reported multiple times both in the APC and PHP bugtrackers, bugs related to this issue include (but are most likely not limited to): http://bugs.php.net/bug.php?id=48787 http://bugs.php.net/bug.php?id=48686 http://pecl.php.net/bugs/bug.php?id=16721 http://pecl.php.net/bugs/bug.php?id=16745 Using a debugger I have found the source of the problem. In my eyes, this is a design problem in PHP's session extension. Rationale: Currently, the session module calls php_session_flush() in its RSHUTDOWN function. This makes sure that the session handler is always called at the very end of execution. However, APC uses its RSHUTDOWN function to remove all class entries from the class table, since they are only shallow copies of memory structures of its cache and the Zend engine should not destroy the class entries by itself. The problem now occurs when the session save handler uses non-internal classes in some way. Since APC, as a dynamic extension, is always loaded AFTER the session module (always a static extension), and the module de-initialization is in reverse order, APC will unload all internal classes *before* the session save handler is executed. So when it is finally the turn of the the session save handler, the classes will no longer be present in the class table, causing the problems described above. Note that APC is not the only extension for which there is a problem with regards to de-initialization. The Spidermonkey extension destroys the current Javascript execution context in the RSHUTDOWN function, so if should any save handler use the Spidermonkey extension, it will not work. In theory, even the date extension could cause minor problems (since the default timezone is reset in RSHUTDOWN), but it gets away since it is always loaded *before* session and thus de-inititialized afterwards. I consider this to be a design problem in PHP's session extension. It tries to use the PHP engine to execute PHP code (the custom session save handler, possibly also the serialization handlers of objects etc.) during a phase where at least some other extension that may be used in that code are already de-initialized. The only reason why it got unnoticed so long is that the RSHUTDOWN code of most - but not all - extensions doesn't really have any really nasty side-effects on its use. It would not surprise me at all, however, if there were memory leaks and other strange this occurring due to this problem. My immediate fix for this problem is to make php_request_shutdown() execute the session save handler just after the userspace shutdown functions but before the RSHUTDOWN functions of the other modules. I have attached short patches to this mail for PHP 5.2, 5.3 and 6.0 that accomplish this. It's not very elegant but it is the least-invasive fix I could come up with. Not existing tests are broken in either of the three branches. Unfortunately, I was not able to create a unit test for this using core PHP alone, since all of the core extensions hide the problem. I wanted to get some feedback first. If nobody objects, however, I'll apply these patches to PHP 5.3 and PHP 6 tomorrow. As for PHP 5.2: I was extremely busy the last two months so I didn't really follow the list. Is it okay if I apply it? I do think this patch should go into the last release of PHP 5.2 (which will be used by quite a few people for a while) since this bug prevents people from using a custom session save handler in combination with APC. On a side note: For PHP6 we may think of adding an additional hook for modules that is executed at the very beginning of the shutdown phase for precisely this purpose. Regards, Christian
Index: ext/session/session.c =================================================================== --- ext/session/session.c (revision 295251) +++ ext/session/session.c (working copy) @@ -36,6 +36,7 @@ #include "php_ini.h" #include "SAPI.h" +#include "php_main.h" #include "php_session.h" #include "ext/standard/md5.h" #include "ext/standard/sha1.h" @@ -2012,7 +2013,9 @@ static PHP_RINIT_FUNCTION(session) /* {{{ */ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ { - php_session_flush(TSRMLS_C); + /* do not flush session, the flush method is assigned to + php_session_pre_shutdown_function in the MINIT, so it will be called + BEFORE execution has ended! */ php_rshutdown_session_globals(TSRMLS_C); return SUCCESS; @@ -2044,6 +2047,10 @@ static PHP_MINIT_FUNCTION(session) /* {{{ */ #ifdef HAVE_LIBMM PHP_MINIT(ps_mm) (INIT_FUNC_ARGS_PASSTHRU); #endif + + /* Flush the session just before shutdown. */ + php_session_pre_shutdown_function = php_session_flush; + return SUCCESS; } /* }}} */ @@ -2059,6 +2066,8 @@ static PHP_MSHUTDOWN_FUNCTION(session) /* {{{ */ ps_serializers[PREDEFINED_SERIALIZERS].name = NULL; memset(&ps_modules[PREDEFINED_MODULES], 0, (MAX_MODULES-PREDEFINED_MODULES)*sizeof(ps_module *)); + php_session_pre_shutdown_function = NULL; + return SUCCESS; } /* }}} */ Index: main/php_main.h =================================================================== --- main/php_main.h (revision 295251) +++ main/php_main.h (working copy) @@ -52,6 +52,8 @@ PHPAPI int php_stream_open_for_zend_ex(const char extern void php_call_shutdown_functions(TSRMLS_D); extern void php_free_shutdown_functions(TSRMLS_D); +extern void (*php_session_pre_shutdown_function)(TSRMLS_D); + /* environment module */ extern int php_init_environ(void); extern int php_shutdown_environ(void); Index: main/main.c =================================================================== --- main/main.c (revision 295251) +++ main/main.c (working copy) @@ -92,6 +92,8 @@ #include "rfc1867.h" /* }}} */ +void (*php_session_pre_shutdown_function)(TSRMLS_D) = NULL; + #ifndef ZTS php_core_globals core_globals; #else @@ -1458,6 +1460,12 @@ void php_request_shutdown(void *dummy) php_call_shutdown_functions(TSRMLS_C); } zend_end_try(); + /* 1.5 Shutdown session if applicable + Execution must still be ongoing because of userspace save handlers. */ + if (PG(modules_activated)) zend_try { + if (php_session_pre_shutdown_function) php_session_pre_shutdown_function(TSRMLS_C); + } zend_end_try(); + /* 2. Call all possible __destruct() functions */ zend_try { zend_call_destructors(TSRMLS_C);
Index: ext/session/session.c =================================================================== --- ext/session/session.c (revision 295254) +++ ext/session/session.c (working copy) @@ -36,6 +36,7 @@ #include "php_ini.h" #include "SAPI.h" +#include "php_main.h" #include "php_session.h" #include "ext/standard/md5.h" #include "ext/standard/sha1.h" @@ -2140,7 +2141,9 @@ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ { int i; - php_session_flush(TSRMLS_C); + /* do not flush session, the flush method is assigned to + php_session_pre_shutdown_function in the MINIT, so it will be called + BEFORE execution has ended! */ php_rshutdown_session_globals(TSRMLS_C); /* this should NOT be done in php_rshutdown_session_globals() */ @@ -2185,6 +2188,10 @@ static PHP_MINIT_FUNCTION(session) /* {{{ */ #ifdef HAVE_LIBMM PHP_MINIT(ps_mm) (INIT_FUNC_ARGS_PASSTHRU); #endif + + /* Flush the session just before shutdown. */ + php_session_pre_shutdown_function = php_session_flush; + return SUCCESS; } /* }}} */ @@ -2200,6 +2207,8 @@ static PHP_MSHUTDOWN_FUNCTION(session) /* {{{ */ ps_serializers[PREDEFINED_SERIALIZERS].name = NULL; memset(&ps_modules[PREDEFINED_MODULES], 0, (MAX_MODULES-PREDEFINED_MODULES)*sizeof(ps_module *)); + php_session_pre_shutdown_function = NULL; + return SUCCESS; } /* }}} */ Index: main/php_main.h =================================================================== --- main/php_main.h (revision 295254) +++ main/php_main.h (working copy) @@ -52,6 +52,8 @@ PHPAPI int php_stream_open_for_zend_ex(const char extern void php_call_shutdown_functions(TSRMLS_D); extern void php_free_shutdown_functions(TSRMLS_D); +extern void (*php_session_pre_shutdown_function)(TSRMLS_D); + /* environment module */ extern int php_init_environ(void); extern int php_shutdown_environ(void); Index: main/main.c =================================================================== --- main/main.c (revision 295254) +++ main/main.c (working copy) @@ -104,6 +104,8 @@ PHPAPI int (*php_register_internal_extensions_func)(TSRMLS_D) = php_register_internal_extensions; +void (*php_session_pre_shutdown_function)(TSRMLS_D) = NULL; + #ifndef ZTS php_core_globals core_globals; #else @@ -1582,6 +1584,12 @@ void php_request_shutdown(void *dummy) php_call_shutdown_functions(TSRMLS_C); } zend_end_try(); + /* 1.5 Shutdown session if applicable + Execution must still be ongoing because of userspace save handlers. */ + if (PG(modules_activated)) zend_try { + if (php_session_pre_shutdown_function) php_session_pre_shutdown_function(TSRMLS_C); + } zend_end_try(); + /* 2. Call all possible __destruct() functions */ zend_try { zend_call_destructors(TSRMLS_C);
Index: ext/session/session.c =================================================================== --- ext/session/session.c (revision 295254) +++ ext/session/session.c (working copy) @@ -38,6 +38,7 @@ #include "SAPI.h" #include "rfc1867.h" #include "php_variables.h" +#include "php_main.h" #include "php_session.h" #include "ext/standard/md5.h" #include "ext/standard/sha1.h" @@ -1968,7 +1969,9 @@ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */ { int i; - php_session_flush(TSRMLS_C); + /* do not flush session, the flush method is assigned to + php_session_pre_shutdown_function in the MINIT, so it will be called + BEFORE execution has ended! */ php_rshutdown_session_globals(TSRMLS_C); /* this should NOT be done in php_rshutdown_session_globals() */ @@ -2015,6 +2018,10 @@ static PHP_MINIT_FUNCTION(session) /* {{{ */ #endif php_session_rfc1867_orig_callback = php_rfc1867_callback; php_rfc1867_callback = php_session_rfc1867_callback; + + /* Flush the session just before shutdown. */ + php_session_pre_shutdown_function = php_session_flush; + return SUCCESS; } /* }}} */ @@ -2030,6 +2037,8 @@ static PHP_MSHUTDOWN_FUNCTION(session) /* {{{ */ ps_serializers[PREDEFINED_SERIALIZERS].name = NULL; memset(&ps_modules[PREDEFINED_MODULES], 0, (MAX_MODULES-PREDEFINED_MODULES)*sizeof(ps_module *)); + php_session_pre_shutdown_function = NULL; + return SUCCESS; } /* }}} */ Index: main/php_main.h =================================================================== --- main/php_main.h (revision 295254) +++ main/php_main.h (working copy) @@ -52,6 +52,8 @@ PHPAPI int php_stream_open_for_zend_ex(const char extern void php_call_shutdown_functions(TSRMLS_D); extern void php_free_shutdown_functions(TSRMLS_D); +extern void (*php_session_pre_shutdown_function)(TSRMLS_D); + /* environment module */ extern int php_init_environ(void); extern int php_shutdown_environ(void); Index: main/main.c =================================================================== --- main/main.c (revision 295254) +++ main/main.c (working copy) @@ -105,6 +105,8 @@ PHPAPI int (*php_register_internal_extensions_func)(TSRMLS_D) = php_register_internal_extensions; +void (*php_session_pre_shutdown_function)(TSRMLS_D) = NULL; + #ifndef ZTS php_core_globals core_globals; #else @@ -1681,6 +1683,12 @@ void php_request_shutdown(void *dummy) zend_call_destructors(TSRMLS_C); } zend_end_try(); + /* 1.5 Shutdown session if applicable + Execution must still be ongoing because of userspace save handlers. */ + if (PG(modules_activated)) zend_try { + if (php_session_pre_shutdown_function) php_session_pre_shutdown_function(TSRMLS_C); + } zend_end_try(); + /* 2. Call all possible shutdown functions registered with register_shutdown_function() */ if (PG(modules_activated)) zend_try { php_call_shutdown_functions(TSRMLS_C);
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php