Refactor and cleanup implementation.

diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c
index 0cdbaf96f9d585a4b59626786b8db2b60ef3843f..9755f0d5c498f66b3b86f392456f33d4ba330f81 100644 (file)
--- a/ext/session/mod_user.c
+++ b/ext/session/mod_user.c
@@ -30,12 +30,20 @@ ps_module ps_mod_user = {
 static void ps_call_handler(zval *func, int argc, zval *argv, zval *retval)
 {
        int i;
+       if (PS(in_save_handler)) {
+               PS(in_save_handler) = 0;
+               ZVAL_UNDEF(retval);
+               php_error_docref(NULL, E_WARNING, "Cannot call save handler function recursive manner");
+               return;
+       }
+       PS(in_save_handler) = 1;
        if (call_user_function(EG(function_table), NULL, func, retval, argc, argv) == FAILURE) {
                zval_ptr_dtor(retval);
                ZVAL_UNDEF(retval);
        } else if (Z_ISUNDEF_P(retval)) {
                ZVAL_NULL(retval);
        }
+       PS(in_save_handler) = 0;
        for (i = 0; i < argc; i++) {
                zval_ptr_dtor(&argv[i]);
        }

diff --git a/ext/session/php_session.h b/ext/session/php_session.h
index da5e48515a43ed5a9a3da27b68c65cae40712cb2..775527c2f61b389cdef826a45d6b107dce88bae4 100644 (file)
--- a/ext/session/php_session.h
+++ b/ext/session/php_session.h
@@ -205,6 +205,7 @@ typedef struct _php_ps_globals {
        zend_bool use_strict_mode; /* whether or not PHP accepts unknown session ids */
        zend_bool lazy_write; /* omit session write when it is possible */
        zend_string *session_vars; /* serialized original session data */
+       zend_bool in_save_handler; /* state that if session is in save handler or not */
 } php_ps_globals;
 
 typedef php_ps_globals zend_ps_globals;

diff --git a/ext/session/session.c b/ext/session/session.c
index 5484390c7e9f8a12165c3f7faf78cdbf7d65113c..ecce0c322cfafe00cb235d366e76f17b64a14134 100644 (file)
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -106,6 +106,7 @@ static inline void php_rinit_session_globals(void) /* {{{ */
        /* TODO: These could be moved to MINIT and removed. These should be initialized by php_rshutdown_session_globals() always when execution is finished. */
        PS(id) = NULL;
        PS(session_status) = php_session_none;
+       PS(in_save_handler) = 0;
        PS(mod_data) = NULL;
        PS(mod_user_is_open) = 0;
        PS(define_sid) = 1;
@@ -2035,7 +2036,7 @@ static PHP_FUNCTION(session_create_id)
                }
        }
 
-       if (PS(session_status) == php_session_active) {
+       if (!PS(in_save_handler) && PS(session_status) == php_session_active) {
                int limit = 3;
                while (limit--) {
                        new_id = PS(mod)->s_create_sid(&PS(mod_data));
@@ -2565,9 +2566,11 @@ static PHP_RSHUTDOWN_FUNCTION(session) /* {{{ */
 {
        int i;
 
-       zend_try {
-               php_session_flush(1);
-       } zend_end_try();
+       if (PS(session_status) == php_session_active) {
+               zend_try {
+                       php_session_flush(1);
+               } zend_end_try();
+       }
        php_rshutdown_session_globals();
 
        /* this should NOT be done in php_rshutdown_session_globals() */

diff --git a/ext/session/tests/bug60634.phpt b/ext/session/tests/bug60634.phpt
index b2f507628729f5364e69dd443961baefac43d930..34786406cb1544f1598bccea74b9d89565b8880b 100644 (file)
--- a/ext/session/tests/bug60634.phpt
+++ b/ext/session/tests/bug60634.phpt
@@ -40,16 +40,19 @@ session_write_close();
 echo "um, hi\n";
 
 /*
-FIXME: Since session module try to write/close session data in
-RSHUTDOWN, write() is executed twices. This is caused by undefined
-function error and zend_bailout(). Current session module codes
-depends on this behavior. These codes should be modified to remove
-multiple write().
-*/
+ * write() calls die(). This results in calling session_flush() which calls
+ * write() in request shutdown. The code is inside save handler still and
+ * calls save close handler.
+ *
+ * Because session_write_close() fails by die(), write() is called twice.
+ * close() is still called at request shutdown since session is active.
+ */
 
 ?>
 --EXPECTF--
 write: goodbye cruel world
-write: goodbye cruel world
-close: goodbye cruel world
 
+Warning: Unknown: Cannot call save handler function recursive manner in Unknown on line 0
+
+Warning: Unknown: Failed to write session data using user defined save handler. (session.save_path: ) in Unknown on line 0
+close: goodbye cruel world

diff --git a/ext/session/tests/bug60634_error_3.phpt b/ext/session/tests/bug60634_error_3.phpt
index b7840b04f9de2b7447458f3db4778869f92c64b1..b38899b8c12d4113a747c9a6881b5f4621c9984b 100644 (file)
--- a/ext/session/tests/bug60634_error_3.phpt
+++ b/ext/session/tests/bug60634_error_3.phpt
@@ -48,4 +48,5 @@ Stack trace:
 #0 [internal function]: write(%s, '')
 #1 {main}
   thrown in %s on line %d
-close: goodbye cruel world
+
+Warning: Unknown: Cannot call save handler function recursive manner in Unknown on line 0

diff --git a/ext/session/tests/bug60634_error_4.phpt b/ext/session/tests/bug60634_error_4.phpt
index 7970b35c7a8059f290be5726a96ef756982e6816..3ac624797475542510bf505995ab32de694c6154 100644 (file)
--- a/ext/session/tests/bug60634_error_4.phpt
+++ b/ext/session/tests/bug60634_error_4.phpt
@@ -48,5 +48,5 @@ Stack trace:
 #0 [internal function]: write('%s', '')
 #1 {main}
   thrown in %s on line %d
-close: goodbye cruel world
 
+Warning: Unknown: Cannot call save handler function recursive manner in Unknown on line 0

diff --git a/ext/session/tests/bug69111.phpt b/ext/session/tests/bug69111.phpt
index c6d10c74a0e6b013e8270885a665d7ff5a40a50e..38201ef64a5278661f84a2983256fa29b050f202 100644 (file)
--- a/ext/session/tests/bug69111.phpt
+++ b/ext/session/tests/bug69111.phpt
@@ -1,7 +1,5 @@
 --TEST--
 Bug #69111 Crash in SessionHandler::read()
---XFAIL--
-It is still a leak
 --SKIPIF--
 <?php include('skipif.inc'); ?>
 --FILE--
@@ -19,4 +17,9 @@ $sh->write("foo", "bar");
 var_dump($sh->read(@$id));
 ?>
 --EXPECTF--
+Warning: SessionHandler::open(): Session is not active in /home/yohgaki/workspace/ext/git/oss/php.net/github-php-src/ext/session/tests/bug69111.php on line 10
+
+Warning: SessionHandler::write(): Session is not active in /home/yohgaki/workspace/ext/git/oss/php.net/github-php-src/ext/session/tests/bug69111.php on line 11
+
+Warning: SessionHandler::read(): Session is not active in /home/yohgaki/workspace/ext/git/oss/php.net/github-php-src/ext/session/tests/bug69111.php on line 12
 bool(false)

diff --git a/ext/session/tests/bug71162.phpt b/ext/session/tests/bug71162.phpt
index fe38ea2999c30c03d3dbda307bfb789361dfca7b..722889d417af797e33c3989dc4a62797308de282 100644 (file)
--- a/ext/session/tests/bug71162.phpt
+++ b/ext/session/tests/bug71162.phpt
@@ -2,12 +2,12 @@
 updateTimestamp never called when session data is empty
 --INI--
 session.use_strict_mode=0
+session.save_handler=files
 --XFAIL--
-Current session module is designed to write empty session always. In addition,
-current session module only supports SessionHandlerInterface only from PHP 7.0.
+Current session module is designed to write empty session always. In addition, current session module only supports SessionHandlerInterface only from PHP 7.0.
 --FILE--
 <?php
-class MySessionHandler implements SessionUpdateTimestampHandlerInterface
+class MySessionHandler extends SessionHandler implements SessionUpdateTimestampHandlerInterface
 {
     public function open($path, $sessname) {
         return TRUE;