Add session_gc()

[yohgaki]

This PR is for RFC
https://wiki.php.net/rfc/session-gc

[laruence]

what does this means? maybe a simple num is better.

[yohgaki]

Fine with me. I'm using nrdels because the name is used for "number of deleted sessions" in the code.

[narfbg]

Doesn't this beat the point of checking gc_probability? If so, shouldn't it be outside of the previous if() condition?

[smalyshev]

Agree, this looks wrong. If you want immediate cleanup, no reason to check probability. Better would be to split the check and cleanup into separate functions and call the latter directly when you need unconditional cleanup.

[yohgaki]

Right it does not needed at all. I'll fix it later :)

[smalyshev]

Why you need session to be active for GC to happen?

[yohgaki]

Because save handler must be activated.

[narfbg]

How so? And can't that be changed?
IMO it's an ugly hack if I have to call session_start() from inside a cronjob.

[yohgaki]

session_gc() handler is defined in save handlers. Unless save handler storage is initialized, gc function won't work.
If you don't like stray sessions by cron tasks, you can use static session ID for the task.

e.g.

However, since this is C code, I can write code that open, gc, close session. In this case, session must be inactive. Otherwise, I'll destroy save handler data structure.

If above code is written. GC cron tack code would be

Modules should do more work rather than users. I'll implement it. Is this satisfactory?

[yohgaki]

@arfbg Andrey, if above code is written

session_start();
session_gc(); // won't work

There may be users write

session_gc();
session_start();

though ;)

[narfbg]

You can still check if the session is already active and don't call close() if that's the case.

[yohgaki]

It may work with many save handlers, but I cannot control how 3rd party/user save handlers are implemented. It's risky. Taking risk does not worth, IMO. I've probably seen too many unexpected usages.

[narfbg]

Pseudo-code:

if (session_status() === PHP_SESSION_ACTIVE)
{
    $sessionHandler->gc();
}
else
{
    $sessionHandler->open();
    $sessionHandler->gc();
    $sessionHandler->close();
}

How can that be a problem?

[smalyshev]

"Static session ID" hack looks very ugly. If we need init happen before we run GC, we should hide it from the user, instead of requiring them to write boilerplate code that makes absolutely no sense for them ("why I need to start dummy session to just run GC?!")

[yohgaki]

I'll document "session_gc() is supposed to be called from periodical task manager such as cron".

IMHO, users shouldn't care extra new session ID created by session_gc() because many useless session IDs are created by cookie disabled browsers anyway, for example.

[laruence]

why zend_long here, and lots of casting below...

[yohgaki]

PHP 7 and up uses zend_long and I would like to get zend_long return value for "num of deleted session" from 3rd party handler in the future.

It could be int for now. If you insist, I don't mind at all using int here.

[laruence]

in that case, why you don't use zend_long num? I don't care about zend_long or int, just seems a little ugly you are doing casting without a good reason.

[yohgaki]

I agree casting is ugly.
I'll simply use zend_long. I'll commit changes soon. Please review.

BTW, sorry for the delay. I didn't see notification mails...

[yohgaki]

I updated patch so that it can simply use zend_long.

User handlers are modified to be able to return LONG or BOOL. User GC functions are supposed to be implemented just like 3rd party C save handlers by returning LONG (0 > for number of deleted session, negative number of errors). FALSE is treated as negative num of deleted sessions. TRUE is treated as 1. Since return value of user handler should not be used by user code, there is no BC.

NOTE: Negative number is used to indicate GC errors for a long time. (IIRC, it's since session module was introduced)

[yohgaki]

merged to PHP-7.1 and master branches