#21306 [Csd->Opn]: partly RSHUTDOWN and "cannot change the session settings" error

ID: 21306 User updated by: Xuefer at 21cn dot com Reported By: Xuefer at 21cn dot com -Status: Closed +Status: Open Bug Type: Session related Operating System: linux PHP Version: 4CVS (2005-09-16) (5CVS, 6CVS) Assigned To: sas New Comment: yeah, all modules is rshutdown correctly. but sorry.. the session error messages is still there, let's check the source PHP_RSHUTDOWN_FUNCTION(session) { zend_try { php_session_flush(TSRMLS_C); <- bailout, function is partly executed, do we care? let's step inside php_rshutdown_session_globals(TSRMLS_C); <- not executed } zend_end_try(); ... } static void php_session_flush(TSRMLS_D) { if(PS(session_status)==php_session_active) { php_session_save_current_state(TSRMLS_C); <- bailout, function is partly executed, but we don't care PS(session_status)=php_session_none; <- not executed. oops! this is important for SESSION_CHECK_ACTIVE_STATE } } i realize that, my patch to zend_API.c make sure every modules's rshutdown handler is called, but can't make sure the rshutdown itself is completed. Previous Comments: ------------------------------------------------------------------------ [2005-09-20 23:04:57] sniper@php.net Fixed in PHP_4_4, PHP_5_1 and HEAD branches with this patch: diff -u -r1.417 session.c --- session.c 3 Aug 2005 14:07:44 -0000 1.417 +++ session.c 20 Sep 2005 20:54:26 -0000 @@ -1807,8 +1807,11 @@ PHP_RSHUTDOWN_FUNCTION(session) { - php_session_flush(TSRMLS_C); - php_rshutdown_session_globals(TSRMLS_C); + zend_try { + php_session_flush(TSRMLS_C); + php_rshutdown_session_globals(TSRMLS_C); + } zend_end_try(); + return SUCCESS; } /* }}} */ Let me know if this doesn't fix the issue. (hard for me to confirm since I can't even reproduce it :) ------------------------------------------------------------------------ [2005-09-20 16:11:44] Xuefer at 21cn dot com about ps_call_handler: hrm.. no idea if any other module/session handler have to be "no bailout" and/or "might bailout". i can only bring the problem up. ------------------------------------------------------------------------ [2005-09-20 16:04:54] Xuefer at 21cn dot com the patch i proposed long ago ([8 Nov 2004 5:24pm CET], the 2nd note in this bug) patches mod_user.c only do u mean here? zend_try { zend_hash_apply(&module_registry, (apply_func_t) module_registry_cleanup TSRMLS_CC); } zend_end_try(); this try/catch here will only make sure zend_deactivate_modules is return correctly, any single bailout will break the loop of zend_hash_apply(), which is foreach item in module_registry, all remaining modules is not RSHUTDOWNed correctly either one of the patches will do. u don't need apply both of them. let module itself, or php engine to make sure not to bailout in the middle. it's up to u guys to decide :) ------------------------------------------------------------------------ [2005-09-20 13:06:37] sniper@php.net And wouldn't it be better to add that stuff in the ps_call_handler() function, not just in one place it is used? ------------------------------------------------------------------------ [2005-09-20 13:04:51] sniper@php.net Why do you patch zend_API.c?? The part you added the zend_try..catch thing is already covered with such elsewhere. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/21306 -- Edit this bug report at http://bugs.php.net/?id=21306&edit=1