Possible PdoSessionStorage issue

[mmucklo]

We seem to have encountered a case where PdoSessionStorage, or a similar class may not save to DB / storage at the end of the request.

We've worked around it, however I'm posting this issue at the request of someone on the newsgroup.

We have a derivative of NativeSessionStorage who's start() method is identical (+/- whitespace) to PdoSessionStorage.php

    /**
     * Starts the session.
     */
    public function start()
    {
        if (self::$sessionStarted) {
            return;
        }

        // use this object as the session handler
        session_set_save_handler(
            array($this, 'sessionOpen'),
            array($this, 'sessionClose'),
            array($this, 'sessionRead'),
            array($this, 'sessionWrite'),
            array($this, 'sessionDestroy'),
            array($this, 'sessionGC')
        );

        parent::start();
    }

We then more or less implement sessionWrite, sessionRead, etc.

The problem is that sessionWrite in certain circumstances doesn't seem to be called at the end of the request, resulting in variables not written to our session storage.

What we see is :

1. PdoSessionStorage(or Equivalent)->sessionWrite() (called by PHP save_handler routine)
2. Session->__destruct
3. Calls Session->save()
4. Calls PdoSessionStorage(NativeSessionStorage)->write()
5. $_SESSION updated
6. end of request. (updated $_SESSION never saved)

What actually needs to happen:

1. Session->__destruct
2. Calls Session->save()
3. Calls PdoSessionStorage(NativeSessionStorage)->write()
4. $_SESSION updated
5. PdoSessionStorage(or Equivalent)->sessionWrite()
6. end of request.

We've hacked around it, but I think the problem is that PHP is calling the save handler before __destruct, and doesn't call it afterwards as perhaps it's cleared or something (don't know internally the order of things).

Our hack extends Session and implementing a __destruct such as below, and using this as the session handler instead by overriding it in a custom session.xml file similar to this:

namespace Us\OurBundle\Session;

class HackedSession
    extends \Symfony\Component\HttpFoundation\Session
{
    public function __destruct()
    {
        parent::__destruct();
        if (true === $this->started && !$this->closed) {
            if(method_exists($this->storage, 'sessionWrite'));
            $this->storage->sessionWrite($this->getId(), session_encode());
        }
    }
}

session.xml:

    <parameters>
                 <!-- ... -->
        <parameter key="session.class">Us\OurBundle\Session\HackedSession</parameter>
    </parameters>

Then load this session.xml under our DependencyInjection class using $loader->load...

Anyway, I'm not sure this is the best way to fix the problem but would appreciate input.

[mvrhov]

Are you by any chance using APC?

[mvrhov]

Addendum does adding session_write_close(); into sessions __destruct class solve the issue?
If it doesn't then we probably need a base Session class which all other session implementations should extend and it's destructor should contain call to that function.

This is coming from: http://si.php.net/manual/en/function.session-set-save-handler.php and this: https://bugs.php.net/bug.php?id=58763

[ghost]

Somewhere in your application, you should register_shutdown_function('session_write_close'); and this should solve the issue. That will ensure the session is closed out properly.

[ghost]

Try this https://github.com/drak/symfony/commit/03400df05bdc2993a63d5e08e7b51162cd22f170

[ghost]

Even better, try placing

register_shutdown_function('session_write_close');

In the constructor of NativeSessionStorage

[mvrhov]

drak it's not ok. We need to apply this to all storage implementations, so if it won't work putting it in save function of Session class then we should have a base class, because it will bite somebody else if done differently.
As I can't simulate that I wanted to have a confirmation before doing an actual PR.

[ghost]

AFAIK, it's just a certain chicken and egg situation with using objects as the session handler because of the way PHP shuts down. The register_shutdown_function('session_write_close') calls whatever has been registered as the save handler when PHP exits. It does this before the session object is destroyed. In fact, doing it this way, there is no need to use __destruct() at all, as PHP will call the save() function.@fabpot

As I remember it, we actually removed our __destruct() entirely to solve this problem (and register the shutdown function as session_write_close).

[mmucklo]

To answer the "Are you by any chance using APC?" question -

no it's actually Memcache, but it was originally supposed to be a MongoDB / Memcache hybrid, with MongoDB acting as the permanent store...However we've disabled the MongoDB part for now for performance reasons.

[ghost]

Try the change in cc07b00 - this is the solution we used to solve this issue in another framework. For reference, the session save handler will always be called when PHP shuts down. There is no need for a __destruct() method because of the we register shutdown function to call the session save handler. This is done outside of the storage driver, in the session class itself so should cover all storage driver cases.

[ghost]

I think I see a problem. Symfony's session save handler is in the Session class, however, when the session save handler is called, it will call the storage driver, not the session since the storage driver's methods are registered. The problem is before the session can be saved, Symfony does some work in Session->save(); which really should be in the storage driver itself...

[mmucklo]

I tried the patch that was suggested, and it didn't work. Basically it ends up saving blank sessions, it seems.

So far we have now something similar to the following:

    $kernel->handle(Request::createFromGlobals())->send();
    // ....
    $container = $kernel->getContainer();
    if (($session = $container->get('session')) && $session->all())
    {
        $session->save();
        $session->close();
        session_write_close();
    }

That and we've put some extra checks in our NativeSessionStorage derivative's sessionWrite function to make sure we don't write the same sessionData twice.

However I did find, pertaining to the original issue that we had a bug in our code

It turns out we had a session_write_close() handler that was being called outside of symfony (you might consider as if it was part of app.php after $kernel->handle).

Removing that made the original __destruct sequence work, with caveats. Basically we found we could no longer reference certain class static variables as the Class objects themselves had been destructed, someone here said it's basically a bad idea to be doing things in a destructor (such as saving to an external session store). Hence we went with the above solution for now...

[ghost]

Yes, the problem is the that the save handlers call code that doesn't do the Session->save() tasks, it calls the storage drivers. I've opened a ticket because the entire Session management is flawed. The only reason things sort of work at the moment is because when PHP shutdown, Session->__destruct() mostly saves the session data, but this is not how sessions are supposed to work. I've explained a lot in #2607 and am working on a PR as there is a lot wrong with the current state of things.

[ghost]

As a temporary work around, you could try adding this into the session class constructor:

register_shutdown_function(array($this, 'save'));

This is not the fix, there are much deeper issues, but it could make things usable for you in the meantime.

[fabpot]

Does the recent PRs merges solve this issue?

[mmucklo]

I apologize for the delay. I did start to take a look at this, but please give me a few more days as I need more time

[ghost]

@mmucklo please remember this is not fixed in Symfony 2.0 but only in the master branch which will become Symfony 2.1

[mmucklo]

drak,

I started reviewing the changes, they seem to make sense. It seems that now you'd now create a SessionHandler extending from NativeSessionHandler if you wanted to do something custom (such as a hybrid memcache + something else approach).

Do you then now override / define a session.handler service in your session.xml (or whatever xml file is being used for dependency injection definitions in your bundle)?