Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73194
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain devilix.net designates 209.85.160.175 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <5324FE40.1070704@sugarcrm.com>
References: <CAPhkiZw5xWRkc=ofh-nB5ZkjfThZFkH_+pK5RZaQFLKhno7u7A@mail.gmail.com>
	<CAGa2bXZiBuw_L_ehvTZb5_UUedtbS0n4GXYUjtKix0S9DX_8KQ@mail.gmail.com>
	<5324FE40.1070704@sugarcrm.com>
Date: Sun, 16 Mar 2014 18:34:31 +0200
Message-ID: <CAPhkiZy1oG_WHijMknB34dyo0aZRDU89EBafpaEGqGM32DCK0A@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: Yasuo Ohgaki <yohgaki@ohgaki.net>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Revert/extend/postpone original RFC about
 read_only, lazy_write sessions
From: narf@devilix.net (Andrey Andreev)

Hi,

On Sun, Mar 16, 2014 at 3:28 AM, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
> Hi!
>
>>> I'm announcing the following RFC for discussion, with the hope that it
>>> can get through before the PHP 5.6 release:
>>> https://wiki.php.net/rfc/session-read_only-lazy_write
>
> Frankly, I'm not sure what is the point of this - submitting RFC that
> says "let's throw out RFC that we just accepted". If somebody didn't
> want that RFC to happen, he'd vote against it, no?

Well, as I said: I don't think the previous RFC was handled properly.
This is probably because it isn't detailed enough for people to
understand it (I for example wouldn't vote for, or against something
that I don't understand). The discussion itself happened during the
holiday season when most people wouldn't care much about work, if at
all - this is also another factor that could've contributed.
Either way, I feel that this is important, so I wrote an RFC.

> I also don't understand what's the problem with read only option. Read
> only option means only one thing - you're going to read the session, but
> not write into it. Hence read only. Your scenario with login check is
> not a problem because all session data are always read at the beginning
> of the session. If another session changes the state later, it in any
> case can not influence the session that have already read the data. The
> only change is that now the session can say "I am not going to change
> the state, nothing I am doing should change the state, so I'm not going
> to waste my time on writing anything back".

Yes, on a basic level the currently accepted 'read_only' matches your
explanation. But there's more to it than that ... options should be
viewed as modes of operation (IMO anyway), and currently this is an
"option" that actually performs an action, and that's not even
obvious. 'read_only' just doesn't have the same meaning as
'read_and_close', which because is an action, should be a separate
function.

As for the login check example, maybe it isn't clear enough. What it
demonstrates is a decision making process, you don't know what happens
afterwards. What if, for example, Request2 deletes a row from a
database at the time of logout and Request1 tries to read the same row
(knowing that it is there because the user is logged in)? The
application breaks.

> Not allowing $_SESSION to be written sounds pretty useless to me, since
> it is not going to be written anywhere anyhow, but this option does not
> prevent implementing it in any way.

How useful that would be is not the point, currently. The problem is
that this is exactly what 'read_only' should mean as an option/mode.
If that name is taken, how would you call it in the future?

> As for lazy_write, incomplete is not the reason to revert accepted
> feature. Anything can be improved and amended, if we rejected features
> because they could be improved, we'd never add anything. There is a use
> case for this feature to exist, as an extension of the read only option
> - it is a case where the session *might* change the state, but not
> always does. So you have three options now:

I didn't mean incomplete in the sense of "it works, but can be
better", but rather as "it only works in half" or "we don't know how
it works in some cases". The RFC itself is incomplete, it doesn't
address a lot of potential issues. It's like knowing how to lock a
door, but not how to unlock it.
I'm not questioning the use cases, don't get me wrong - I want this
feature. It just looks like a prototype and not a real solution.

> 1. My request is never going to change the state => use read_only
> 2. My request might change the state and thus I need exclusive lock =>
> use lazy_write
> 3. My request always changes the state => use defaults
>
> The difference between 1 and 2 is shorter period under lock, between 2
> and 3 - saving time on transmitting data over the wire that are already
> in session storage anyway. As for UPDATE_FUNC, I understand this is only
> for updating "last used" if it exists and may be defaulted to nothing.

"last used", "last updated", "last modified", "last accessed" ...
however you call it, it always exists in some form. Without such a
timestamp, there's no way to decide if a session has expired or not,
so you can't default to nothing ... Something has to update it and
that something may be added to "stock" session handlers, but doesn't
exist in third-party or userland ones - this is where it breaks.

> Again, nothing prevents us in PHP 6 from changing the defaults and
> making this option part of the default option set.

I didn't say there is, just that it's more appropriate to leave it for then.

Cheers,
Andrey.