Hi,

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.

2. What happens if SessionHandler::gc() was triggered by the probability
mechanism, but you then call session_gc() manually?

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.

3. If session_gc() bypasses the gc_probability, gc_divisor INI settings*,
then why does it still rely on gc_maxlifetime?

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.

Cheers,
Andrey.

Reply via email to