Hi Andrey,

On Fri, Apr 8, 2016 at 12:12 AM, Andrey Andreev <n...@devilix.net> wrote:
> On Thu, Apr 7, 2016 at 9:05 AM, Yasuo Ohgaki <yohg...@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
yohg...@ohgaki.net

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to