Deadlock when using session_start(array("read_and_close"=>true));

[huguesalary]

In PHP 7, session_start() can take an $options array parameter (php doc)

Using this parameter with Cm_RedisSession will cause the read() function to increment the lock field of the redis hash structure, then close() will be immediately called.

Unfortunately, close() does not decrement the lock field, which means that any subsequent call to read() will wait until the configured timeout before taking the lock on the session.

    /**
     * Overridden to prevent calling getLifeTime at shutdown
     *
     * @return bool
     */
    public function close()
    {
        $this->_log("Closing connection");
        if ($this->_redis) $this->_redis->close();
        return true;
    }

Submitted PR #17, however, I'm not convinced this is the right way of handling this issue.

It might make more sense to add logic to read() to release the lock when being used with "read_and_close".

[huguesalary]

Thinking back about this, the read() function should simply not grab a lock at all when read_and_close is set. Unfortunately, I couldn't find a way to detect whether read_and_close has been set or not.

[colinmollenhour]

I dunno, read_and_close does not necessarily imply that locking should be skipped.. There could be cases where you want it to lock still and if you want to disable locking at least for this handler you can bypass locking in other ways. So, I think the close method should unlock the session like you did in your PR but I think rather than reading the pid from the session a boolean flag could be used like $this->_isSessionUnlocked so that if close is called before write then it will unlock the session and otherwise it will do nothing different than before.

Reading the docs, how does "lazy_write" not break the locking as well? Does it mean that if session data is not changed it will also just call open, read, close? I suppose in Magento the session data is always updated so maybe that's why this hasn't been reported as a bug yet..

[colinmollenhour]

Oh, there is already a _sessionWritten flag. It would mean one more RTT, but perhaps write should only write the data and close should release the lock?

[huguesalary]

I dunno, read_and_close does not necessarily imply that locking should be skipped.. There could be cases where you want it to lock still

I'm not sure there's any reason you would want to get a lock on read. If you're in a read_and_close mode, you'll read the session but close it immediately when the call to read() returns. What would getting the lock useful for?

[colinmollenhour]

Perhaps not in the case of Magento but in a more strict environment it may be that requests should be serialized. I'm not sure how the default files handler handles read_and_close but it does use locking. I think requiring serialized requests is not a good way to design but if you want pure performance there is the 'disable_locking' flag.