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")

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.

> 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

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.

Then you should definitely raise the issue on the internals list. I've read through a lot of emails about this but I'm still not certain why the RFC's code is pending - they might be able to resolve it, or at least explain what its current state is.

This is a backwards incompatible change.
For reasons very similar to the original submitter, this has broken our application.

Strongly consider updating your migration notes as well as the original documentation:
http://php.net/manual/en/migration56.incompatible.php
http://lt1.php.net/manual/en/function.session-set-save-handler.php

> 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.
That's not a problem. Its fine for it to be optional but in 5.6 its NOT optional and there's no way to turn off lazy writing. The RFC said there would be, but there's not.

> 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(). 
That's also fine, but was not implemented. As the RFC was implemented, there is no way to turn off the lazy write functionality.

If there was ANY way to turn off this functionality (besides rebuilding PHP without the mentioned comment, which is what GS is doing now), then this wouldn't be an issue at all. Because of that, I'd consider this to be a bug. The RFC was fine in that it introduced multiple workarounds but that was not implemented, which is why it is breaking applications.

I don't understand the reason why you need "write" even when session data was not changed at all.

BTW, I was suggesting "update" handler for the RFC introduce this feature, but there were objection and withdrawned.

I may try to add user space "update" again.

> I don't understand the reason why you need "write" even when session data was not changed at all.

Two reasons I brought up on the internals thread:
- We would like to update the timestamp of the session.
- We have some data that isn't in $_SESSION that needs to be written. We don't use $_SESSION because we need the data to be indexable (not serialized) and didn't want to duplicate the data in two places.

> BTW, I was suggesting "update" handler for the RFC introduce this feature, but there were objection and withdrawned.

I'm unaware of the full objection to that idea; however, I think calling the write() custom handlers until an alternative is implemented would suffice to fix this issue. I understand that doing the lazy write call was an optimization for normal sessions but this change can break some custom session implementations.

> I may try to add user space "update" again.

Based on what was said on the internals thread it sounds like this isn't an option for PHP 5.6. I assume you mean something can be done for 5.7 and for PHP 5.6 the commit will just have to be reverted?

Since your are using user save handler, I think this could be fixed as a bug. I have to check the code to be sure, though.

Hmm. The commit you're referring is nothing to do with you issue. It was reverted and new PR is there for this. (It seems should be adjusted and committed)

https://github.com/yohgaki/php-src/compare/PHP-5.6-rfc-session-lock

It isn't merged yet. So lazy write patch is unrelated at all.

I've tried see if issue could be reproduced, but it cannot.
Could you write full/working reproducible script?

I found what's wrong. Since lazy write patch was in discussion and I left WIP patch in the repo and left this patch merge to RM. This must be addressed ASAP.