On Thu, Mar 13, 2014 at 10:09 PM, Yasuo Ohgaki <[email protected]> wrote:
> Hi Andrey,
>
> On Fri, Mar 14, 2014 at 1:56 AM, Andrey Andreev <[email protected]> wrote:
>>
>> Another 2 items that didn't go through the RFC process or any discussion:
>
>
> These are covered RFC discussions and/or some other discussions in this
> list.
>
>>
>> - Implemented Request #20421 (session_abort() and session_reset()
>> function). (Yasuo)
>>
>> https://bugs.php.net/bug.php?id=20421
>>
>> Does this change mtime of the session?
>> I also have other (rather philosophical) arguments against this, but I
>> don't want to get deeper into it now ... it should just be reverted
>> together with session_serializer_name(), session_gc() on the basis of
>> no discussion at all.
>
>
> abort and reset were discussed in RFC discussion. It was't a long
> discussion though. Since this is just a simple API addition, it added
> simply.
>
> What's your reason?
http://markmail.org/message/om3ofpjlit7iq6d2
Is this the discussion you're talking about? If so, session_abort()
and session_reset() are barely mentioned in it, and no wonder - it was
during the holidays.
That's not a productive discussion ...
>> - Implemented Request #17860 (Session write short circuit). (Yasuo)
>>
>> https://bugs.php.net/bug.php?id=17860
>>
>> The problem with that is that there's no API exposed for custom save
>> handlers to decide when to only update mtime or the whole session. I
>> think the question was exactly about that, actually.
>> It is also not clear how is that implemented and for which of the
>> available session handlers ... is it all of them?
>
>
> This one is discussed in following RFC, since it affects existing behavior.
> Some one pointed out and the RFC was created.
The RFC mentions a 'lazy_write' option and voting ended at the end of February.
The feature (not optional) was committed way earlier.
>> Then later (note that it *already is implemented*), it was voted as
>> part of this RFC, which made it optional:
>>
>> https://wiki.php.net/rfc/session-lock-ini
>>
>> This RFC is broken to begin with ... what ended up as accepted from
>> this RFC was actually very far from the main intention behind it (and
>> it silently accepted passing ini options to session_start() with no
>> explicit voting for that - just saying, otherwise I like that
>> feature).
>> Also, there's no description for "read_only" except the obvious - that
>> it only reads. It should at least mention that a shared lock can be
>> used for that case.
>>
>> IMO, if this feature had gone through proper discussion instead of
>> just presenting a yes/no option as part of another change, we'd end up
>> with a different solution.
>>
>> Actually, I'd like to ask if it's possible for an RFC to override a
>> decision taken from a previous one? And can it be done quick? Or if
>> not - can that be postponed for 5.7?
>> The lazy_write option is nice, but it's not optimal and it would be
>> embarassing if it makes it into 5.6 only to be later reversed in favor
>> of a better solution. What I'm thinking of is a session_is_changed()
>> function and "lazy_write" at all times (there's no reason for that to
>> be optional).
>
>
> You should comment during discussion. There was long discussion for this.
Sorry, I wasn't around during that discussion.
That's why I'm asking if a new RFC can override the old one.
> Shared lock could be used easily for some session storage, but it requires
> save handlers modification and it could be implemented _independent_ of
> read_only. Web apps must return response ASAP. Therefore, any locks that
> could be avoided should be able to be avoided including read(shared) lock.
I'm talking about handlers that PHP itself ships, they all implement
locking (exclusive locking).
An exclusive lock can only be obtained by one process and all others
will have to wait for it to be freed.
A shared lock can be acquired by multiple processes and as such, is
concurrency-friendly. However, it is only usable if sessions are in
read_only mode, hence why I'm suggesting it.
> Anyway, read_lock is confusing unless save handler API supports it. i.e.
> User cannot know if it really support read_lock like current
> use_strict_mode.
Users needs to know if they are in read_only mode.
Currently they can't, because the RFC proposed it an obscure setting
available only to session_start(). If it was an INI setting, it
could've been readable.
> Why not write a RFC for read_lock? I'm OK to implement it if you would like.
Because this is one of the few session-related things that doesn't
need an RFC, but rather common sense.
It's just an implementation detail that was not described in the RFC.
----------
All I'm saying is, this whole thing wasn't handled properly and/or
with the required attention.
It's nobody's fault, but it ended up broken and I want to fix it
(possibly with the help of somebody to write the patch, because I'm
not a C programmer).
I'll write an RFC on it shortly and I hope it can get through before
the release.
Cheers,
Andrey.