Edit report at https://bugs.php.net/bug.php?id=68331&edit=1
ID: 68331
Comment by: jay at grooveshark dot com
Reported by: mark at grooveshark dot com
Summary: Session custom storage callable functions not being
called
Status: Not a bug
Type: Bug
Package: Session related
Operating System: All
PHP Version: 5.6.2
Block user comment: N
Private report: N
New Comment:
Fellow Grooveshark developer here. Our session handler adds some OOP functionality, the reasons for
which are outside the scope of this discussion, but the reason this change causes problems for us is
that PHP does not always know when the session data has changed, because some of our session data is
not part of $_SESSION. Our custom save handler already checks to see if a write is actually
necessary, so this optimization not only causes this bug for us where write is not being called when
we expect it to be, but it also doesn't optimize anything for us. If this functionality was
behind a flag we could easily turn it off, but for now we are having to choose between rewriting the
way our session handler works or recompiling PHP without the code in that commit.
Previous Comments:
------------------------------------------------------------------------
[2014-10-31 20:03:32] requinix@php.net
> what is the point of having an RFC if the results are ignored by resolving a> feature request bug with a finite feature from the RFC?
The RFC is not being ignored. Might be out of the spotlight at the moment but as far as I know
there's still every intention on it being implemented.
Besides, as you saw the change happened before the RFC.
> The feature request didn't even ask for what was implemented
It suggested a flag, but the core issue was that sessions were being rewritten unnecessarily and
that PHP should check for changes so that the write could be skipped. @yohgaki implemented exactly
that.
> which for some reason is still assigned to PHP 4.2.1 in the bug tracker
The field is for what version the bug is being filed against, not when it is fixed.
> The documentation does not state that storage handlers won't be called in> some cases.
True, and I agree that it would be a good thing to mention.
> This results in custom session implementations having to do the writing in> close() with a bunch of convoluted logic to get the serialized session data> from read() or write().
If you want to make write() rewrite the session regardless of changes, apparently? The point is that
you don't have to do that.
The RFC reintroduces the idea of lazy writing as an optional feature so when that is in then
you'll be able to (not) use it.
> The inability to have the timestamp updated via write() on a custom session> is a bug in my opinion. This would be fine if there were something like> updateTimestamp as an alternative.
Personally I prefer this lazy writing. write() can update a modification timestamp, open() or
close() can update an access timestamp, and you GC based on access time.
With that said, the RFC (well, the code in its PR) does introduce a supplemental interface
"SessionUpdateTimestampHandler" which adds a method "updateTimestamp" that would
be called during lazy writing instead of write(). So that should address your concerns.
Putting aside your (very understandable) confusion regarding the lazy writing change and the RFC, I
NABed this because the change was made intentionally as a resolution to an old feature request. Now
that you've explained that it's not just a matter of an unexpected change but that it
causes a problem for you, though it still seems an avoidable one to me, I'd reconsider.
However the RFC still resolves that. I'm not entirely sure of its status, but like I said I
think it's still intended to be merged in at some point. There might have been an outstanding
issue preventing it from being merged into 5.6? If you're not averse to mailing lists then I
suggest asking on internals to find out for sure what's happened to it.
http://php.net/mailing-lists.php
------------------------------------------------------------------------
[2014-10-31 15:07:23] mark at grooveshark dot com
Sorry about linking to the RFC incorrectly; however, what is the point of having an RFC if the
results are ignored by resolving a feature request bug with a finite feature from the RFC? The
feature request didn't even ask for what was implemented but rather just for a flag that a
session is dirty. The unfortunate part is that the RFC was created after the partial feature was
implemented (which for some reason is still assigned to PHP 4.2.1 in the bug tracker).
I feel like calling this not a bug is a mistake. The documentation does not state that storage
handlers won't be called in some cases. This results in custom session implementations having
to do the writing in close() with a bunch of convoluted logic to get the serialized session data
from read() or write(). The inability to have the timestamp updated via write() on a custom session
is a bug in my opinion. This would be fine if there were something like updateTimestamp as an
alternative.
Happy Halloween! Hope you have a good weekend.
------------------------------------------------------------------------
[2014-10-31 02:36:01] requinix@php.net
The state of the RFC and its code is a bit complicated. Suffice it to say that its changes are not
implemented yet.
The commit you found is regarding an old request which is separate from, though did play an
inspirational part in, the RFC.
https://bugs.php.net/bug.php?id=17860
As such, the fact that the session handler (file or user or otherwise) is not called when there have
not been changes to the data is intentional.
Side note: the change was mentioned for 5.6.0.
http://php.net/ChangeLog-5.php#5.6.0
(in the Session section as "Session write short circuit")
------------------------------------------------------------------------
[2014-10-31 00:43:52] mark at grooveshark dot com
Description:
------------
Call to write are not being triggered when setting up custom handlers for sessions using
session_set_save_handler if the underlying session data hasn't changed. It looks like this has
been done by design for regular session handling but was calls were suppose to happen for custom
session handling.
There wasn't really any documentation for this change in 5.6 but we found the commit and RFC
that caused this bug.
Here is the commit that caused the bug: https://github.com/php/php-src/commit/554021d21e1b2517313a377676260c188152c2eb#diff-52eb9eb7f9d5d9125fbb1337a6541c06R549
The RFC discussing the topic is here: https://wiki.php.net/rfc/session-lock-ini
It looks like read_only and lazy_write were accepted for this RFC.
Test script:
---------------
http://gobin.io/QfTs?php
Expected result:
----------------
Session's write custom handler should be called.
Actual result:
--------------
The custom write handler is not called.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=68331&edit=1