Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92154
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.68 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAPhkiZx7zc1=1zyx=vJvZtr7x7OhktDFCHMK4GdOCD_DSqbKCg@mail.gmail.com>
References: <CAGa2bXb-jON8qZyje_ETckyO-KCmcbhTjcJt=NJZ1agAgzqMoQ@mail.gmail.com>
 <CAPhkiZx7zc1=1zyx=vJvZtr7x7OhktDFCHMK4GdOCD_DSqbKCg@mail.gmail.com>
Date: Fri, 8 Apr 2016 12:46:48 +0900
Message-ID: <CAGa2bXaZnsGnrKAAPmxHG+hut3+1=8bdOER--WrsXXowoiXu=A@mail.gmail.com>
To: Andrey Andreev <narf@devilix.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC][Discussion] Add session_gc()
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Andrey,

On Fri, Apr 8, 2016 at 12:12 AM, Andrey Andreev <narf@devilix.net> wrote:
> On Thu, Apr 7, 2016 at 9:05 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>>
>> Hi all,
>>
>> This is very old RFC that adds session_gc().
>> https://wiki.php.net/rfc/session-gc
>>
>> It simply adds missing session_gc() function.
>> Comments are appreciated.
>>
>
> There are a few question marks raised by the RFC's (lack of) content ...
>
> 1. How exactly does the function work?
>
> I can see that in the patch, but it should be explained in the RFC too ..
> even the function signature is missing.

Added

>
> 2. What happens if SessionHandler::gc() was triggered by the probability
> mechanism, but you then call session_gc() manually?

It should perform GC w/o condition

>
> It seems odd that you've put "Document calling session_gc() periodically is
> the best practice" in the RFC itself, but are not addressing this issue.
> IMO, it is more important to document this and here's an idea: trigger at
> least an E_NOTICE in that case.
>
> Speaking of the documentation part - it's not hard to imagine A LOT of
> people doing this:
>
>     session_start();
>     session_gc();
>
> That's not a small problem.

Shooting their own foot is not our problem, but leaving obsolete and
possibly active session is ours.


>
> 3. If session_gc() bypasses the gc_probability, gc_divisor INI settings*,
> then why does it still rely on gc_maxlifetime?

It's a mistake. I spend too little time for this.
This will be fixed.

>
> The signature for SessionHandlerInterface::gc() accepts a TTL value as a
> parameter; session_gc() can too:
>
>     function session_gc($ttl = ini_get('session.gc_maxlifetime')) {}
>
> * Note: The current patch doesn't actually ignore gc_probability, but I
> think that's by mistake (also relevant to my previous point). I'll comment
> on the PR on GitHub.

Good point.
Since session module does the job if module should execute GC or not.
It should be 1 or very short time.

These are addressed soon.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net