Hello,
I just saw this in the 5.6-alpha1 changelog:
- Implemented Request #54649 (Create session_serializer_name()). (Yasuo)
... - Implemented Request #11100 (session_gc() function). (Yasuo)
I didn't find an RFC and I don't know a way to search for a discussion
on internals (Google doesn't find one).
session_serializer_name(): https://bugs.php.net/bug.php?id=54649
I don't think that this should've been implemented, especially with
the reasoning from the feature request ... The same could be applied
to all session ini options and especially now since session_start()
accepts them, it's useless IMO.
session_gc() was added in a similar fashion:
https://bugs.php.net/bug.php?id=11100
There's already a comment about how the same thing can be achieved by
overloading SessionHandler::gc(), but a user could also just alter
session.gc_divisor, session.gc_probability to ensure that the garbage
collector is started.
And while session_serializer_name() is just redundant, session_gc()
could cause performance issues.
I'd strongly suggest that these 2 functions should be removed before
PHP 5.6 is officially released.
Regards,
Andrey Andreev.
Hi Andrey,
I just saw this in the 5.6-alpha1 changelog:
- Implemented Request #54649 (Create session_serializer_name()). (Yasuo)
...- Implemented Request #11100 (session_gc() function). (Yasuo)
I didn't find an RFC and I don't know a way to search for a discussion
on internals (Google doesn't find one).session_serializer_name(): https://bugs.php.net/bug.php?id=54649
I don't think that this should've been implemented, especially with
the reasoning from the feature request ... The same could be applied
to all session ini options and especially now sincesession_start()
accepts them, it's useless IMO.
session_gc()was added in a similar fashion:
https://bugs.php.net/bug.php?id=11100There's already a comment about how the same thing can be achieved by
overloading SessionHandler::gc(), but a user could also just alter
session.gc_divisor, session.gc_probability to ensure that the garbage
collector is started.
Users want to have contorl when gc is performed rather than luck. This is
reasonable. IMO.
And while session_serializer_name() is just redundant,
session_gc()
could cause performance issues.
There is session_module_name(), why not session_serializer_name()?
Many 3rd party save handler implements many serializer.
session_gc() is nothing to do with performance. Users may abuse. The same
argument applies to gc probability.
I'd strongly suggest that these 2 functions should be removed before
PHP 5.6 is officially released.
These are simple additional APIs.
If your argument applies, there should be number of new RFCs.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Yasuo,
Hi Andrey,
I just saw this in the 5.6-alpha1 changelog:
- Implemented Request #54649 (Create session_serializer_name()).
(Yasuo)
...- Implemented Request #11100 (session_gc() function). (Yasuo)
I didn't find an RFC and I don't know a way to search for a discussion
on internals (Google doesn't find one).session_serializer_name(): https://bugs.php.net/bug.php?id=54649
I don't think that this should've been implemented, especially with
the reasoning from the feature request ... The same could be applied
to all session ini options and especially now sincesession_start()
accepts them, it's useless IMO.
session_gc()was added in a similar fashion:
https://bugs.php.net/bug.php?id=11100There's already a comment about how the same thing can be achieved by
overloading SessionHandler::gc(), but a user could also just alter
session.gc_divisor, session.gc_probability to ensure that the garbage
collector is started.Users want to have contorl when gc is performed rather than luck. This is
reasonable. IMO.
I like control, it's nice and I already have it:
ini_set('session.gc_probability', 1);
ini_set('session.gc_divisor', 1);
And while session_serializer_name() is just redundant,
session_gc()
could cause performance issues.There is
session_module_name(), why not session_serializer_name()?
Why not session_cookie_lifetime()?
Why not session_cache_limiter()?
Why not session_entropy_file()?
etc.
"Why not" is never a good argument in programming and really, when is
the last time that you've seen something accepted here on internals
because of "why not"?
I'd rather question if session_module_name() is useful, or any
function that's an alias for ini_set('something', ...).
Many 3rd party save handler implements many serializer.
I haven't seen many. In fact, I've seen none.
session_gc()is nothing to do with performance. Users may abuse. The same
argument applies to gc probability.
The problem is, users WILL abuse and that's bad.
Think of what the manual page would say about that function. Something
in the lines of:
Calls the session garbage collector, ignoring
session.gc_probablity and session.gc_divisor.
What does that say to an inexperienced and even to the intermediate
user, who doesn't even know what GC is?
"Hey, this collects garbage! It's nice to clean things up, I
should always call it!"
Yet another bad practice spreads.
Sure, that could apply to gc_probability, gc_divisor, but those are
necessary options and they are not easy to find unless you are
specifically looking for them.
I'd strongly suggest that these 2 functions should be removed before
PHP 5.6 is officially released.These are simple additional APIs.
If your argument applies, there should be number of new RFCs.
Actually, if my argument applies - both feature requests should've
been ignored. Lots of people make "this would be nice" feature
requests without having the knowledge and experience to know if it
would really be "nice". That's why there's a bunch of smart people
here to take the decisions for them, collectively. I myself have been
forced to write an RFC for a smaller and less impactful change, but
that's not the point. Both of these functions should've at least been
discussed here on internals.
And finally, do you assume that there's demand for these features?
session_gc() specifically was requested 13 YEARS ago and it didn't
even get a "+1" comment in that timeframe. There are people on this
list who'd question user demand even if people are tweeting about the
feature with fingers crossed that it would be implemented ... these
are two that hardly anybody cares about.
Cheers,
Andrey.
Hi Andrey,
Users want to have contorl when gc is performed rather than luck. This
is
reasonable. IMO.I like control, it's nice and I already have it:
ini_set('session.gc_probability', 1); ini_set('session.gc_divisor', 1);
Of course you can.
It does not look nice as API. API should have good name for it. IMO.
session_gc() is useful for both low traffic site and high traffic site.
I don't see reason not to have explicit gc API. User may simply call
session_gc();
periodically by cron, etc.
And while session_serializer_name() is just redundant,
session_gc()
could cause performance issues.There is
session_module_name(), why not session_serializer_name()?Why not session_cookie_lifetime()?
Why notsession_cache_limiter()?
Why not session_entropy_file()?
etc.
Those aren't changed by programs often.
As I wrote, 3rd party save handler implements their own serializers.
Not like above example, it is likely called with session_module_name(). e.g.
session_module_name('memcache');
session_serializer_name('igbinary');
It looks better than
session_module_name('memcached');
ini_set('session.serializer_handler', 'igbinary');
It's reasonable to have session_serializer_name(), since it would be used
with session_module_name(). API should have good name here, too.
We may discuss what we should do with module functions that modify it's
INI. There are number of them already. I think you need RFC for this.
My opinion is "There should be API with obvious name, if it is used often
or mandatory".
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi,
Hi Andrey,
Users want to have contorl when gc is performed rather than luck. This
is
reasonable. IMO.I like control, it's nice and I already have it:
ini_set('session.gc_probability', 1); ini_set('session.gc_divisor', 1);Of course you can.
It does not look nice as API. API should have good name for it. IMO.
session_gc()is useful for both low traffic site and high traffic site.
I don't see reason not to have explicit gc API. User may simply call
session_gc();periodically by cron, etc.
There's a very good reason not to have it, you just ignored most of my
previous email which explained exactly that.
And while session_serializer_name() is just redundant,
session_gc()
could cause performance issues.There is
session_module_name(), why not session_serializer_name()?Why not session_cookie_lifetime()?
Why notsession_cache_limiter()?
Why not session_entropy_file()?
etc.Those aren't changed by programs often.
Oh, trust me - cookie lifetime is changed way more often than the serializer.
And how do you define often anyway? Sessions are something that are
setup once, it's rarely the case that a programmer needs to change any
of the session INI settings more than once in the same program.
As I wrote, 3rd party save handler implements their own serializers.
Not like above example, it is likely called withsession_module_name(). e.g.session_module_name('memcache');
session_serializer_name('igbinary');
It's your opinion that this is "likely" done that way. I for one have
never used session_module_name().
And again, this is not something that people would write more often
than once in the whole application.
It looks better than
session_module_name('memcached');
ini_set('session.serializer_handler', 'igbinary');
It doesn't look better than:
ini_set('session.save_handler', 'memcached');
ini_set('session.serialize_handler', 'igbinary');
IMO, this is more explicit and more understandable for people who
haven't read the manuals for the "custom" functions.
It's reasonable to have session_serializer_name(), since it would be used
withsession_module_name(). API should have good name here, too.
I'm again questioning whether session_module_name() is
useful/necessary in the first place.
We may discuss what we should do with module functions that modify it's INI.
There are number of them already. I think you need RFC for this.
On this, I agree.
My opinion is "There should be API with obvious name, if it is used often or
mandatory".
On this - not. ;)
Cheers,
Andrey.
Hi Andrey,
Hi Andrey,
On Wed, Mar 12, 2014 at 8:54 AM, Andrey Andreev narf@devilix.net
wrote:Users want to have contorl when gc is performed rather than luck.
This
is
reasonable. IMO.I like control, it's nice and I already have it:
ini_set('session.gc_probability', 1); ini_set('session.gc_divisor', 1);Of course you can.
It does not look nice as API. API should have good name for it. IMO.
session_gc()is useful for both low traffic site and high traffic site.
I don't see reason not to have explicit gc API. User may simply call
session_gc();periodically by cron, etc.
There's a very good reason not to have it, you just ignored most of my
previous email which explained exactly that.
Users may abuse INI or API.
If you worry about abuse, it would be better to have API and warn users.
Users should purge deleted session data for security reasons. It is
especially important for low traffic sites. High traffic sites may need
periodic purge due
varying request rate. Not having GC API is API design bug.
If there should not be convenient API, file_get/put_contents() aren't
needed neither.
And while session_serializer_name() is just redundant,
session_gc()could cause performance issues.
There is
session_module_name(), why not session_serializer_name()?Why not session_cookie_lifetime()?
Why notsession_cache_limiter()?
Why not session_entropy_file()?
etc.Those aren't changed by programs often.
Oh, trust me - cookie lifetime is changed way more often than the
serializer.
It's bad practice, indeed.
User should not change default 0 (session cookie - i.e. cookie only lives
in browser's memory and destroyed by browser termination)
If users need auto login or like, they should implement it properly by
using setcookie(). (i.e. Never use session ID cookie for long life session)
And how do you define often anyway? Sessions are something that are
setup once, it's rarely the case that a programmer needs to change any
of the session INI settings more than once in the same program.As I wrote, 3rd party save handler implements their own serializers.
Not like above example, it is likely called withsession_module_name().
e.g.session_module_name('memcache');
session_serializer_name('igbinary');It's your opinion that this is "likely" done that way. I for one have
never usedsession_module_name().
And again, this is not something that people would write more often
than once in the whole application.
It does not matter much if you use often or not.
As an API design, not having session_serializer_name() is strange.
It looks better than
session_module_name('memcached');
ini_set('session.serializer_handler', 'igbinary');It doesn't look better than:
ini_set('session.save_handler', 'memcached');
ini_set('session.serialize_handler', 'igbinary');
I don't mind having discussion deprecating all API that modifying its INI.
I'm some where between +1 and 0 for this.
IMO, this is more explicit and more understandable for people who
haven't read the manuals for the "custom" functions.
It's reasonable to have session_serializer_name(), since it would be used
withsession_module_name(). API should have good name here, too.I'm again questioning whether
session_module_name()is
useful/necessary in the first place.
Now I think I understand your point of view.
We may discuss what we should do with module functions that modify it's
INI.There are number of them already. I think you need RFC for this.
On this, I agree.
Consistency matters. I'm trying to be consistent with current way.
I'm tends to agree deprecating all INI modifying APIs ;)
Let's have the RFC, I'll vote +1 for it.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi again,
Hi Andrey,
Hi Andrey,
On Wed, Mar 12, 2014 at 8:54 AM, Andrey Andreev narf@devilix.net
wrote:Users want to have contorl when gc is performed rather than luck.
This
is
reasonable. IMO.I like control, it's nice and I already have it:
ini_set('session.gc_probability', 1); ini_set('session.gc_divisor', 1);Of course you can.
It does not look nice as API. API should have good name for it. IMO.
session_gc()is useful for both low traffic site and high traffic site.
I don't see reason not to have explicit gc API. User may simply call
session_gc();periodically by cron, etc.
There's a very good reason not to have it, you just ignored most of my
previous email which explained exactly that.Users may abuse INI or API.
If you worry about abuse, it would be better to have API and warn users.
I don't think you understand what my point is ... the API itself must
be hard to abuse.
The INI settings are not that often misused because they are hard to
find for anybody that's not specifically looking for them. This is not
the case for functions and exposing the GC as a function is something
that would at least triple the abuse.
Users should purge deleted session data for security reasons. It is
especially important for low traffic sites. High traffic sites may need
periodic purge due varying request rate.
I don't disagree with that, but it's not what we're discussing.
Not having GC API is API design bug.
There is an API, you're introducing a second one.
If there should not be convenient API, file_get/put_contents() aren't needed
neither.
Nonsense. This is not even worth explaining why.
And while session_serializer_name() is just redundant,
session_gc()
could cause performance issues.There is
session_module_name(), why not session_serializer_name()?Why not session_cookie_lifetime()?
Why notsession_cache_limiter()?
Why not session_entropy_file()?
etc.Those aren't changed by programs often.
Oh, trust me - cookie lifetime is changed way more often than the
serializer.It's bad practice, indeed.
User should not change default 0 (session cookie - i.e. cookie only lives in
browser's memory and destroyed by browser termination)If users need auto login or like, they should implement it properly by using
setcookie(). (i.e. Never use session ID cookie for long life session)
I was just giving you an example of how your assumption is wrong,
auto-login is not the subject of this discussion.
And how do you define often anyway? Sessions are something that are
setup once, it's rarely the case that a programmer needs to change any
of the session INI settings more than once in the same program.As I wrote, 3rd party save handler implements their own serializers.
Not like above example, it is likely called withsession_module_name().
e.g.session_module_name('memcache');
session_serializer_name('igbinary');It's your opinion that this is "likely" done that way. I for one have
never usedsession_module_name().
And again, this is not something that people would write more often
than once in the whole application.It does not matter much if you use often or not.
As an API design, not having session_serializer_name() is strange.
You just changed your argumentation from "it's often needed" to "it
doesn't matter if it's often used, but not having it is strange".
Again, my stance is that the existance session_module_name() is
strange, not the other way around.
It looks better than
session_module_name('memcached');
ini_set('session.serializer_handler', 'igbinary');It doesn't look better than:
ini_set('session.save_handler', 'memcached');
ini_set('session.serialize_handler', 'igbinary');I don't mind having discussion deprecating all API that modifying its INI.
I'm some where between +1 and 0 for this.IMO, this is more explicit and more understandable for people who
haven't read the manuals for the "custom" functions.It's reasonable to have session_serializer_name(), since it would be
used
withsession_module_name(). API should have good name here, too.I'm again questioning whether
session_module_name()is
useful/necessary in the first place.Now I think I understand your point of view.
We may discuss what we should do with module functions that modify it's
INI.
There are number of them already. I think you need RFC for this.On this, I agree.
Consistency matters. I'm trying to be consistent with current way.
I'm tends to agree deprecating all INI modifying APIs ;)
Let's have the RFC, I'll vote +1 for it.
So ... you'd vote +1 for deprecating all similar functions, yet you
just added another one without even discussing it? :)
That would be a nice RFC indeed, but I still stand behind my initial
suggestion - both of these functions should be excluded from the 5.6
release.
Cheers,
Andrey.
Hi Andrey,
Hi Andrey,
On Wed, Mar 12, 2014 at 8:04 PM, Andrey Andreev narf@devilix.net
wrote:On Wed, Mar 12, 2014 at 2:19 AM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:Hi Andrey,
On Wed, Mar 12, 2014 at 8:54 AM, Andrey Andreev narf@devilix.net
wrote:Users want to have contorl when gc is performed rather than luck.
This
is
reasonable. IMO.I like control, it's nice and I already have it:
ini_set('session.gc_probability', 1); ini_set('session.gc_divisor', 1);Of course you can.
It does not look nice as API. API should have good name for it. IMO.
session_gc()is useful for both low traffic site and high traffic
site.
I don't see reason not to have explicit gc API. User may simply call
session_gc();periodically by cron, etc.
There's a very good reason not to have it, you just ignored most of my
previous email which explained exactly that.Users may abuse INI or API.
If you worry about abuse, it would be better to have API and warn users.I don't think you understand what my point is ... the API itself must
be hard to abuse.
The INI settings are not that often misused because they are hard to
find for anybody that's not specifically looking for them. This is not
the case for functions and exposing the GC as a function is something
that would at least triple the abuse.Users should purge deleted session data for security reasons. It is
especially important for low traffic sites. High traffic sites may need
periodic purge due varying request rate.I don't disagree with that, but it's not what we're discussing.
The root cause is design bug of GC.
We may fix this with other method. This would be the best solution.
It's possible that save handlers use session.expire setting to execute GC
periodically.
And while session_serializer_name() is just redundant,
session_gc()could cause performance issues.
There is
session_module_name(), why not session_serializer_name()?Why not session_cookie_lifetime()?
Why notsession_cache_limiter()?
Why not session_entropy_file()?
etc.Those aren't changed by programs often.
Oh, trust me - cookie lifetime is changed way more often than the
serializer.It's bad practice, indeed.
User should not change default 0 (session cookie - i.e. cookie only
lives in
browser's memory and destroyed by browser termination)If users need auto login or like, they should implement it properly by
using
setcookie(). (i.e. Never use session ID cookie for long life session)I was just giving you an example of how your assumption is wrong,
auto-login is not the subject of this discussion.
This is the common abuse of session. IMO.
And how do you define often anyway? Sessions are something that are
setup once, it's rarely the case that a programmer needs to change any
of the session INI settings more than once in the same program.As I wrote, 3rd party save handler implements their own serializers.
Not like above example, it is likely called with
session_module_name().
e.g.session_module_name('memcache');
session_serializer_name('igbinary');It's your opinion that this is "likely" done that way. I for one have
never usedsession_module_name().
And again, this is not something that people would write more often
than once in the whole application.It does not matter much if you use often or not.
As an API design, not having session_serializer_name() is strange.You just changed your argumentation from "it's often needed" to "it
doesn't matter if it's often used, but not having it is strange".
Again, my stance is that the existancesession_module_name()is
strange, not the other way around.
"often or mandatory"
I think it's both.
It looks better than
session_module_name('memcached');
ini_set('session.serializer_handler', 'igbinary');It doesn't look better than:
ini_set('session.save_handler', 'memcached');
ini_set('session.serialize_handler', 'igbinary');I don't mind having discussion deprecating all API that modifying its
INI.
I'm some where between +1 and 0 for this.IMO, this is more explicit and more understandable for people who
haven't read the manuals for the "custom" functions.It's reasonable to have session_serializer_name(), since it would be
used
withsession_module_name(). API should have good name here, too.I'm again questioning whether
session_module_name()is
useful/necessary in the first place.Now I think I understand your point of view.
We may discuss what we should do with module functions that modify
it's
INI.
There are number of them already. I think you need RFC for this.On this, I agree.
Consistency matters. I'm trying to be consistent with current way.
I'm tends to agree deprecating all INI modifying APIs ;)
Let's have the RFC, I'll vote +1 for it.So ... you'd vote +1 for deprecating all similar functions, yet you
just added another one without even discussing it? :)That would be a nice RFC indeed, but I still stand behind my initial
suggestion - both of these functions should be excluded from the 5.6
release.
No problem.
It would be easier to understand if you explain your point ;)
I'll fix root cause of GC issue later. GC should not depend on luck.
Are you going to write RFC for INI issue?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Yasuo,
I'll fix root cause of GC issue later. GC should not depend on luck.
What do you mean you'll fix it? What would the solution to that be?
Are you going to write RFC for INI issue?
Not at this time, it won't make it into 5.6 anyway.
Julien Pauli already suggested that we should discuss that for PHP6
... I have some ideas, I'll start a discussion about them soon. :)
Cheers,
Andrey.
Hi Andrey,
I'll fix root cause of GC issue later. GC should not depend on luck.
What do you mean you'll fix it? What would the solution to that be?
Save handler may keep & check last GC time. If it's over session.expire,
perform GC.
Issue is that "session data that should be deleted" is not deleted and
could be alive long term due to current design. Current design is good
enough for many usage, but it is not the best practice obviously.
"Keep & check last GC time" requires additional overhead for every request.
This is the reason why I prefer to have session_gc() and encourage users to
use it properly.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Am 13.03.2014 02:41 schrieb "Yasuo Ohgaki" yohgaki@ohgaki.net:
Save handler may keep & check last GC time. If it's over session.expire,
perform GC.
That would have a "scoping" problem: session.expire, at least in theory,
can be different for different scripts / usages of session in the same
application, but such a global last-GC timestamp would be... global.
Also generally GC is a costly global operation, slowing down the request
that hits it (probably that's why you envision a cronjob right?)
Issue is that "session data that should be deleted" is not deleted and
could be alive long term due to current design. Current design is good
The proper solution, would be to keep a timestamp stored with each
session. kind of a last-modified timestamp. (expiry does work off last
modification, right?). Anyhow, the trick is to check at session_start
whether that timestamp plus whatever session.expire is set at, already is
in the past - and when that happens, destroy the one session on storage and
act as if it hadn't been found.
This way there is no longer any correctness problem, and a GC is only
needed to clean up sessions that are never requested again - and for that I
fully agree that a separate - e.g. cron driven - "GC now" job would be best
(not affecting latency of running frontent requests too much).
Of course, going to such a solution, requires a change to all session
implementations in that they must provide for storage and return of such a
last-modified timestamp (or othewise the improvement cannot be realized for
a non-updated-to-the-new-scheme session handler)
best regards
Patrick
Hi Patrick,
Save handler may keep & check last GC time. If it's over session.expire,
perform GC.That would have a "scoping" problem: session.expire, at least in theory,
can be different for different scripts / usages of session in the same
application, but such a global last-GC timestamp would be... global.Also generally GC is a costly global operation, slowing down the request
that hits it (probably that's why you envision a cronjob right?)
Yes. This is issue for save handlers that should perform costly GC.
Issue is that "session data that should be deleted" is not deleted and
could be alive long term due to current design. Current design is goodThe proper solution, would be to keep a timestamp stored with each
session. kind of a last-modified timestamp. (expiry does work off last
modification, right?). Anyhow, the trick is to check at session_start
whether that timestamp plus whatever session.expire is set at, already is
in the past - and when that happens, destroy the one session on storage and
act as if it hadn't been found.This way there is no longer any correctness problem, and a GC is only
needed to clean up sessions that are never requested again - and for that I
fully agree that a separate - e.g. cron driven - "GC now" job would be best
(not affecting latency of running frontent requests too much).Of course, going to such a solution, requires a change to all session
implementations in that they must provide for storage and return of such a
last-modified timestamp (or othewise the improvement cannot be realized for
a non-updated-to-the-new-scheme session handler)
I agree. It's proper and feasible solution. I've sent issues about
session_regenerate_id() in other thread. It could be solved by time stamp
stored as session data also. Storing time stamp out side of $_SESSION data
would be cleanest solution, but it requires new interface.
It may be good idea to write a new save handler interface while keeping
older one.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
It may be good idea to write a new save handler interface while keeping
older one.
It would really helpful to target a later versions, maybe 6 and see
what we can do while minimizing the impact in userlands (whether or
not custom session handlers are implemented).
But all the discussions and RFCs about basically the same needs have
been very confusing.
What do you think? Is it something you could consider?
--
Pierre
@pierrejoye | http://www.libgd.org
Hi,
The proper solution, would be to keep a timestamp stored with each session. kind of a last-modified timestamp. (expiry does work off last modification, right?). Anyhow, the trick is to check at session_start whether that timestamp plus whatever session.expire is set at, already is in the past - and when that happens, destroy the one session on storage and act as if it hadn't been found.
Such a timestamp already exists, you can't implement sessions without
one because there would be no way to know when a session have expired.
For the files handler, it's filemtime() (afaik) and the behavior that
you described is probably already in use (I see no reason not to).
However, that's not in the scope of GC.
a GC is only needed to clean up sessions that are never requested again - and for that I fully agree that a separate - e.g. cron driven - "GC now" job would be best (not affecting latency of running frontent requests too much).
^ That is what a garbage collector is, indeed. :)
It may be good idea to write a new save handler interface while keeping
older one.It would really helpful to target a later versions, maybe 6 and see
what we can do while minimizing the impact in userlands (whether or
not custom session handlers are implemented).But all the discussions and RFCs about basically the same needs have
been very confusing.What do you think? Is it something you could consider?
I am deffinately a proponent of that. I already mentioned earlier that
I have some ideas ... this is one of them.
However, while Yasuo seems to always be keen on creating RFCs
immediately, I'd rather focus on the existing ones that are session
related:
https://wiki.php.net/rfc/secure-session-options-by-default
https://wiki.php.net/rfc/session_regenerate_id
5.6 is really close and this is important stuff that was supposed to
go in, but has not had updates recently and lagged behind.
Changes for future versions can wait a few days/weeks ...
Cheers,
Andrey.
Hi again,
Another 2 items that didn't go through the RFC process or any discussion:
- Implemented Request #20421 (session_abort() and
session_reset()
function). (Yasuo)
https://bugs.php.net/bug.php?id=20421
Does this change mtime of the session?
I also have other (rather philosophical) arguments against this, but I
don't want to get deeper into it now ... it should just be reverted
together with session_serializer_name(), session_gc() on the basis of
no discussion at all.
-
Implemented Request #17860 (Session write short circuit). (Yasuo)
The problem with that is that there's no API exposed for custom save
handlers to decide when to only update mtime or the whole session. I
think the question was exactly about that, actually.
It is also not clear how is that implemented and for which of the
available session handlers ... is it all of them?
Then later (note that it already is implemented), it was voted as
part of this RFC, which made it optional:
https://wiki.php.net/rfc/session-lock-ini
This RFC is broken to begin with ... what ended up as accepted from
this RFC was actually very far from the main intention behind it (and
it silently accepted passing ini options to session_start() with no
explicit voting for that - just saying, otherwise I like that
feature).
Also, there's no description for "read_only" except the obvious - that
it only reads. It should at least mention that a shared lock can be
used for that case.
IMO, if this feature had gone through proper discussion instead of
just presenting a yes/no option as part of another change, we'd end up
with a different solution.
Actually, I'd like to ask if it's possible for an RFC to override a
decision taken from a previous one? And can it be done quick? Or if
not - can that be postponed for 5.7?
The lazy_write option is nice, but it's not optimal and it would be
embarassing if it makes it into 5.6 only to be later reversed in favor
of a better solution. What I'm thinking of is a session_is_changed()
function and "lazy_write" at all times (there's no reason for that to
be optional).
Regards,
Andrey.
Hi Andrey,
Another 2 items that didn't go through the RFC process or any discussion:
These are covered RFC discussions and/or some other discussions in this
list.
- Implemented Request #20421 (session_abort() and
session_reset()
function). (Yasuo)https://bugs.php.net/bug.php?id=20421
Does this change mtime of the session?
I also have other (rather philosophical) arguments against this, but I
don't want to get deeper into it now ... it should just be reverted
together with session_serializer_name(),session_gc()on the basis of
no discussion at all.
abort and reset were discussed in RFC discussion. It was't a long
discussion though. Since this is just a simple API addition, it added
simply.
What's your reason?
Implemented Request #17860 (Session write short circuit). (Yasuo)
The problem with that is that there's no API exposed for custom save
handlers to decide when to only update mtime or the whole session. I
think the question was exactly about that, actually.
It is also not clear how is that implemented and for which of the
available session handlers ... is it all of them?
This one is discussed in following RFC, since it affects existing behavior.
Some one pointed out and the RFC was created.
Then later (note that it already is implemented), it was voted as
part of this RFC, which made it optional:
https://wiki.php.net/rfc/session-lock-iniThis RFC is broken to begin with ... what ended up as accepted from
this RFC was actually very far from the main intention behind it (and
it silently accepted passing ini options tosession_start()with no
explicit voting for that - just saying, otherwise I like that
feature).
Also, there's no description for "read_only" except the obvious - that
it only reads. It should at least mention that a shared lock can be
used for that case.IMO, if this feature had gone through proper discussion instead of
just presenting a yes/no option as part of another change, we'd end up
with a different solution.Actually, I'd like to ask if it's possible for an RFC to override a
decision taken from a previous one? And can it be done quick? Or if
not - can that be postponed for 5.7?
The lazy_write option is nice, but it's not optimal and it would be
embarassing if it makes it into 5.6 only to be later reversed in favor
of a better solution. What I'm thinking of is a session_is_changed()
function and "lazy_write" at all times (there's no reason for that to
be optional).
You should comment during discussion. There was long discussion for this.
Shared lock could be used easily for some session storage, but it requires
save handlers modification and it could be implemented independent of
read_only. Web apps must return response ASAP. Therefore, any locks that
could be avoided should be able to be avoided including read(shared) lock.
Anyway, read_lock is confusing unless save handler API supports it. i.e.
User cannot know if it really support read_lock like current
use_strict_mode.
Why not write a RFC for read_lock? I'm OK to implement it if you would like.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
Another 2 items that didn't go through the RFC process or any discussion:
These are covered RFC discussions and/or some other discussions in this
list.
- Implemented Request #20421 (session_abort() and
session_reset()
function). (Yasuo)https://bugs.php.net/bug.php?id=20421
Does this change mtime of the session?
I also have other (rather philosophical) arguments against this, but I
don't want to get deeper into it now ... it should just be reverted
together with session_serializer_name(),session_gc()on the basis of
no discussion at all.abort and reset were discussed in RFC discussion. It was't a long
discussion though. Since this is just a simple API addition, it added
simply.What's your reason?
http://markmail.org/message/om3ofpjlit7iq6d2
Is this the discussion you're talking about? If so, session_abort()
and session_reset() are barely mentioned in it, and no wonder - it was
during the holidays.
That's not a productive discussion ...
Implemented Request #17860 (Session write short circuit). (Yasuo)
The problem with that is that there's no API exposed for custom save
handlers to decide when to only update mtime or the whole session. I
think the question was exactly about that, actually.
It is also not clear how is that implemented and for which of the
available session handlers ... is it all of them?This one is discussed in following RFC, since it affects existing behavior.
Some one pointed out and the RFC was created.
The RFC mentions a 'lazy_write' option and voting ended at the end of February.
The feature (not optional) was committed way earlier.
Then later (note that it already is implemented), it was voted as
part of this RFC, which made it optional:https://wiki.php.net/rfc/session-lock-iniThis RFC is broken to begin with ... what ended up as accepted from
this RFC was actually very far from the main intention behind it (and
it silently accepted passing ini options tosession_start()with no
explicit voting for that - just saying, otherwise I like that
feature).
Also, there's no description for "read_only" except the obvious - that
it only reads. It should at least mention that a shared lock can be
used for that case.IMO, if this feature had gone through proper discussion instead of
just presenting a yes/no option as part of another change, we'd end up
with a different solution.Actually, I'd like to ask if it's possible for an RFC to override a
decision taken from a previous one? And can it be done quick? Or if
not - can that be postponed for 5.7?
The lazy_write option is nice, but it's not optimal and it would be
embarassing if it makes it into 5.6 only to be later reversed in favor
of a better solution. What I'm thinking of is a session_is_changed()
function and "lazy_write" at all times (there's no reason for that to
be optional).You should comment during discussion. There was long discussion for this.
Sorry, I wasn't around during that discussion.
That's why I'm asking if a new RFC can override the old one.
Shared lock could be used easily for some session storage, but it requires
save handlers modification and it could be implemented independent of
read_only. Web apps must return response ASAP. Therefore, any locks that
could be avoided should be able to be avoided including read(shared) lock.
I'm talking about handlers that PHP itself ships, they all implement
locking (exclusive locking).
An exclusive lock can only be obtained by one process and all others
will have to wait for it to be freed.
A shared lock can be acquired by multiple processes and as such, is
concurrency-friendly. However, it is only usable if sessions are in
read_only mode, hence why I'm suggesting it.
Anyway, read_lock is confusing unless save handler API supports it. i.e.
User cannot know if it really support read_lock like current
use_strict_mode.
Users needs to know if they are in read_only mode.
Currently they can't, because the RFC proposed it an obscure setting
available only to session_start(). If it was an INI setting, it
could've been readable.
Why not write a RFC for read_lock? I'm OK to implement it if you would like.
Because this is one of the few session-related things that doesn't
need an RFC, but rather common sense.
It's just an implementation detail that was not described in the RFC.
All I'm saying is, this whole thing wasn't handled properly and/or
with the required attention.
It's nobody's fault, but it ended up broken and I want to fix it
(possibly with the help of somebody to write the patch, because I'm
not a C programmer).
I'll write an RFC on it shortly and I hope it can get through before
the release.
Cheers,
Andrey.
Hi Andrey,
Shared lock could be used easily for some session storage, but it
requires
save handlers modification and it could be implemented independent of
read_only. Web apps must return response ASAP. Therefore, any locks that
could be avoided should be able to be avoided including read(shared)
lock.I'm talking about handlers that PHP itself ships, they all implement
locking (exclusive locking).An exclusive lock can only be obtained by one process and all others
will have to wait for it to be freed.
A shared lock can be acquired by multiple processes and as such, is
concurrency-friendly. However, it is only usable if sessions are in
read_only mode, hence why I'm suggesting it.
read_only mode is "read and finish session".
Shared lock is "can read session, but not write".
Semantics differs.
Anyway, read_lock is confusing unless save handler API supports it. i.e.
User cannot know if it really support read_lock like current
use_strict_mode.Users needs to know if they are in read_only mode.
Currently they can't, because the RFC proposed it an obscure setting
available only tosession_start(). If it was an INI setting, it
could've been readable.
I like INIs. Therefore, it was INI initially.
It was made to start option, since others do not like INIs.
Why not write a RFC for read_lock? I'm OK to implement it if you would
like.Because this is one of the few session-related things that doesn't
need an RFC, but rather common sense.
It's just an implementation detail that was not described in the RFC.
My common sense tells that there should be session_gc(), there should be
session_serializer_name() if there is session_module_name ;)
However, I like the idea of getting rid of INI modifying functions. I'm OK
with either
- have both
session_module_name()and session_serializer_name() - getting rid of them all
The RFC was made with long discussions. It involves other RFC, too. I may
miss to write some and it may be difficult to understand reason behind it.
I'll answer if there is questions. Please don't forget to write your reason
why feature should be changed.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
All I'm saying is, this whole thing wasn't handled properly and/or
with the required attention.
It's nobody's fault, but it ended up broken and I want to fix it
(possibly with the help of somebody to write the patch, because I'm
not a C programmer).
I think shared lock is useful. It requires additional work to change save
handler interface and save handler code. It's a distinct feature from
read_only. You can get status via session_status() if you need to know
during execution. i.e. If session is started with read_only, you'll
get PHP_SESSION_NONE status.
I suppose read_only is good enough name for the feature. I appreciate
better name for it, if there is.
Although I would not like to propose/implement the RFC now, but I may
implement it if RFC passes.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi,
I think shared lock is useful. It requires additional work to change save
handler interface and save handler code. It's a distinct feature from
read_only. You can get status viasession_status()if you need to know
during execution. i.e. If session is started with read_only, you'll get
PHP_SESSION_NONEstatus.
This is a bug.
A read-only session is not a non-existing session.
I'm thinking more of something like a SessionHandler::$isReadOnly property.
I suppose read_only is good enough name for the feature. I appreciate better
name for it, if there is.
It is a perfect name, no need to change it.
Although I would not like to propose/implement the RFC now, but I may
implement it if RFC passes.
Great, I'll count on that. :)
Hi Andrey,
I think shared lock is useful. It requires additional work to change save
handler interface and save handler code. It's a distinct feature from
read_only. You can get status viasession_status()if you need to know
during execution. i.e. If session is started with read_only, you'll get
PHP_SESSION_NONEstatus.This is a bug.
A read-only session is not a non-existing session.
This is not a bug.
session_start(['read_only'=>true])
is the same as
session_start();
session_commit();
It's much faster because no additional API call nor write to session
storage.
I'm thinking more of something like a SessionHandler::$isReadOnly property.
I suppose read_only is good enough name for the feature. I appreciate
better
name for it, if there is.It is a perfect name, no need to change it.
Although I would not like to propose/implement the RFC now, but I may
implement it if RFC passes.Great, I'll count on that. :)
I have to think about the use case if it's really useful, but I may
implement shared lock if RFC passes.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Yasuo,
Hi Andrey,
I think shared lock is useful. It requires additional work to change
save
handler interface and save handler code. It's a distinct feature from
read_only. You can get status viasession_status()if you need to know
during execution. i.e. If session is started with read_only, you'll get
PHP_SESSION_NONEstatus.This is a bug.
A read-only session is not a non-existing session.This is not a bug.
session_start(['read_only'=>true])
is the same as
session_start();
session_commit();It's much faster because no additional API call nor write to session
storage.
This is broken, consider the following (multiple tab/ajax/whatever
concurrency) scenario:
Request1: session_start(['read_only' => TRUE]);
Request2: session_start(); unset($_SESSION['logged_in']); session_commit();
Request1: still logged in
^ This screams "danger".
It's also, redundant ... do you really believe you'd get +1 votes for
that feature if the voters were really aware that this would be just
an alias for (sesson_start() && session_commit()) ?
I don't think so. This is simply not what "read only" means which
makes this whole thing very misleading and if people who were supposed
to review the feature got confused by it, imagine what happens in
userland.
Cheers,
Andrey.
Am 14.03.2014 11:07 schrieb "Andrey Andreev" narf@devilix.net:
This is broken, consider the following (multiple tab/ajax/whatever
concurrency) scenario:Request1: session_start(['read_only' => TRUE]);
Request2:session_start(); unset($_SESSION['logged_in']);
session_commit();
Request1: still logged in^ This screams "danger".
That is not broken WRT sessions themselves, because the readonly session
of request 1 will not be permitted to change session data (it is readonly)
and as such the situation is the same as if request 1 completed before
request 2 started.
It is broken when the session lock is "misused" as a lock wrt. other,
nonsession data / database modifications / whatever else. In that case,
using the readonly session feature or any other kind of different-lifetime
session lock scheme is bad for the application - don't do that then.
best regards
Patrick
Hi,
Am 14.03.2014 11:07 schrieb "Andrey Andreev" narf@devilix.net:
This is broken, consider the following (multiple tab/ajax/whatever
concurrency) scenario:Request1: session_start(['read_only' => TRUE]);
Request2:session_start(); unset($_SESSION['logged_in']);
session_commit();
Request1: still logged in^ This screams "danger".
That is not broken WRT sessions themselves, because the readonly session
of request 1 will not be permitted to change session data (it is readonly)
and as such the situation is the same as if request 1 completed before
request 2 started.It is broken when the session lock is "misused" as a lock wrt. other,
nonsession data / database modifications / whatever else. In that case,
using the readonly session feature or any other kind of different-lifetime
session lock scheme is bad for the application - don't do that then.
It is not broken functionally, indeed.
It's broken by design - if I write session_start(), options or not, I
would never expect it to immediately close the session. It's highly
misleading and this will lead to a lot of abuse.
Cheers,
Andrey.
Hi all,
It is not broken functionally, indeed.
It's broken by design - if I writesession_start(), options or not, I
would never expect it to immediately close the session. It's highly
misleading and this will lead to a lot of abuse.
I don't want to confuse users.
Better name would be appreciated.
Perhaps, "close" may be better option name.
session_start(['read_only'=>true]);
↓
session_start(['close']=>true);
Any comments/better names?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi,
Hi all,
It is not broken functionally, indeed.
It's broken by design - if I writesession_start(), options or not, I
would never expect it to immediately close the session. It's highly
misleading and this will lead to a lot of abuse.I don't want to confuse users.
Better name would be appreciated.Perhaps, "close" may be better option name.
session_start(['read_only'=>true]);
↓
session_start(['close']=>true);Any comments/better names?
I'd rather suggest this to be a separate function and not an option
for session_start(). I've got this coverered in a draft RFC that I
will announce for discussion later today.
Now back to the main topic:
Please exclude session_serializer_name(), session_gc(),
session_reset(), session_abort() and the "session write short-circuit"
from the 5.6 branch.
Cheers,
Andrey.
Hi Andrey,
I'd rather suggest this to be a separate function and not an option
forsession_start(). I've got this coverered in a draft RFC that I
will announce for discussion later today.
It could have dedicated function, but it is not needed.
Now back to the main topic:
Please exclude session_serializer_name(),
session_gc(),
session_reset(),session_abort()and the "session write short-circuit"
from the 5.6 branch.
I removed session_serializer_name() and session_gc()
(Although session_gc() is mandatory API, IMO)
I like the idea removing INI modifying function in the future release.
I don't understand reason why you insist removal of session_reset()
and session_abort(). They are just missing API for session module,
like session_gc().
There should be API (i.e. function/method or parameters) for distinct
operations that user may use.
I may agree if you could provide the reason why there should not be
these APIs.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi all,
Now back to the main topic:
Please exclude session_serializer_name(),
session_gc(),
session_reset(),session_abort()and the "session write short-circuit"
from the 5.6 branch.I removed session_serializer_name() and
session_gc()
(Althoughsession_gc()is mandatory API, IMO)
I like the idea removing INI modifying function in the future release.I don't understand reason why you insist removal of
session_reset()
andsession_abort(). They are just missing API for session module,
likesession_gc().There should be API (i.e. function/method or parameters) for distinct
operations that user may use.I may agree if you could provide the reason why there should not be
these APIs.
I thought it might be better to explain what new functions do.
session_gc() executes GC without tweaking INIs.
session_abort() aborts session without writing $_SESSION. There is no way
achieve w/o it.
session_reset() re-reads session and re-initializes $_SESSION. There is no
way achieve w/o it. (It could be done with session_abort(), though)
"write short circuit" omits "write" when $_SESSION hasn't change. There is
no point calling write API and writing to storage for the same data.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi,
Hi all,
Now back to the main topic:
Please exclude session_serializer_name(),
session_gc(),
session_reset(),session_abort()and the "session write short-circuit"
from the 5.6 branch.I removed session_serializer_name() and
session_gc()
(Althoughsession_gc()is mandatory API, IMO)
I like the idea removing INI modifying function in the future release.I don't understand reason why you insist removal of
session_reset()
andsession_abort(). They are just missing API for session module,
likesession_gc().There should be API (i.e. function/method or parameters) for distinct
operations that user may use.I may agree if you could provide the reason why there should not be
these APIs.
session_abort(), session_reset() are just two functions that somebody
asked about in 2002 via bugs.php.net and nobody responded (11 years
without a single comment) right up until you just assumed it's a good
idea and commited it. If it was "missing API", surely at least 1 user
per year would've requested it. ;)
I understand that you see them as small, trivial additions, but being
a part of sessions automatically makes them very important and as
such, they should be evaluated collectively, not by a single person.
I thought it might be better to explain what new functions do.
session_gc()executes GC without tweaking INIs.
session_abort()aborts session without writing $_SESSION. There is no way
achieve w/o it.
session_reset()re-reads session and re-initializes $_SESSION. There is no
way achieve w/o it. (It could be done withsession_abort(), though)
"write short circuit" omits "write" when $_SESSION hasn't change. There is
no point calling write API and writing to storage for the same data.
"write short circuit" as I understand it, is an exact copy of the
'lazy_write' option. This will be addressed in the previously
mentioned RFC that I'll post later today.
Cheers,
Andrey.
Hi all,
On Sat, Mar 15, 2014 at 1:15 PM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:Now back to the main topic:
Please exclude session_serializer_name(),
session_gc(),
session_reset(),session_abort()and the "session write short-circuit"
from the 5.6 branch.I removed session_serializer_name() and
session_gc()
(Althoughsession_gc()is mandatory API, IMO)
I like the idea removing INI modifying function in the future release.I don't understand reason why you insist removal of
session_reset()
andsession_abort(). They are just missing API for session module,
likesession_gc().There should be API (i.e. function/method or parameters) for distinct
operations that user may use.I may agree if you could provide the reason why there should not be
these APIs.
session_abort(),session_reset()are just two functions that somebody
asked about in 2002 via bugs.php.net and nobody responded (11 years
without a single comment) right up until you just assumed it's a good
idea and commited it. If it was "missing API", surely at least 1 user
per year would've requested it. ;)I understand that you see them as small, trivial additions, but being
a part of sessions automatically makes them very important and as
such, they should be evaluated collectively, not by a single person.
It does not explain why it is not needed.
For example, session_abort() can be used to with error/exception during
execution.
You may not use, but I would use it to make sure $_SESSION would not
contains
any unneeded changes when error/exception occurred.
I thought it might be better to explain what new functions do.
session_gc()executes GC without tweaking INIs.
session_abort()aborts session without writing $_SESSION. There is no way
achieve w/o it.
session_reset()re-reads session and re-initializes $_SESSION. There is
no
way achieve w/o it. (It could be done withsession_abort(), though)
"write short circuit" omits "write" when $_SESSION hasn't change. There
is
no point calling write API and writing to storage for the same data."write short circuit" as I understand it, is an exact copy of the
'lazy_write' option. This will be addressed in the previously
mentioned RFC that I'll post later today.
No. It's not.
"lazy_write" does not lock session data while "write short circuit" does.
In other words, "lazy_write" changes session behavior, but "write short
circuit" does not.
i.e. With locked session data, "write short circuit" would not change how
session manager works.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi,
Hi all,
On Sat, Mar 15, 2014 at 1:15 PM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:Now back to the main topic:
Please exclude session_serializer_name(),
session_gc(),
session_reset(),session_abort()and the "session write short-circuit"
from the 5.6 branch.I removed session_serializer_name() and
session_gc()
(Althoughsession_gc()is mandatory API, IMO)
I like the idea removing INI modifying function in the future release.I don't understand reason why you insist removal of
session_reset()
andsession_abort(). They are just missing API for session module,
likesession_gc().There should be API (i.e. function/method or parameters) for distinct
operations that user may use.I may agree if you could provide the reason why there should not be
these APIs.
session_abort(),session_reset()are just two functions that somebody
asked about in 2002 via bugs.php.net and nobody responded (11 years
without a single comment) right up until you just assumed it's a good
idea and commited it. If it was "missing API", surely at least 1 user
per year would've requested it. ;)I understand that you see them as small, trivial additions, but being
a part of sessions automatically makes them very important and as
such, they should be evaluated collectively, not by a single person.It does not explain why it is not needed.
I'll just repeat what I've said previously: acceptance of a new
feature can't be justified by a lack of "why not" reasons, it should
work the other way around.
For example,
session_abort()can be used to with error/exception during
execution.
You may not use, but I would use it to make sure $_SESSION would not
contains
any unneeded changes when error/exception occurred.
I also want to use a lot of features that don't currently exist in
PHP, but only few people like yourself have the privilege of being
able to commit them. However, everything should go through peer review
before it gets in, not after.
"write short circuit" omits "write" when $_SESSION hasn't change. There
is
no point calling write API and writing to storage for the same data."write short circuit" as I understand it, is an exact copy of the
'lazy_write' option. This will be addressed in the previously
mentioned RFC that I'll post later today.No. It's not.
"lazy_write" does not lock session data while "write short circuit" does.
In other words, "lazy_write" changes session behavior, but "write short
circuit" does not.
i.e. With locked session data, "write short circuit" would not change how
session manager works.
I can't stress enough how confusing and potentially unsafe that is.
Cheers,
Andrey.
Hi Andrey,
You need to argue why missing APIs that are added aren't needed.
Just because they were missing years will not make them unneeded.
For example, there are few new function requests for mbstring. Just adding
missing APIs to NEW release does not require RFC, simply making thing
more efficient and faster does not require RFC, making small internal BC
does
not require RFC, does it?
You are asking to make RFC always. (I didn't say discussion isn't needed)
Anyone can make pull requests now. I even send PR if there needs to have
some agreement. Anyone can comment committed patch.
"write short circuit" omits "write" when $_SESSION hasn't change.
There
is
no point calling write API and writing to storage for the same data."write short circuit" as I understand it, is an exact copy of the
'lazy_write' option. This will be addressed in the previously
mentioned RFC that I'll post later today.No. It's not.
"lazy_write" does not lock session data while "write short circuit" does.
In other words, "lazy_write" changes session behavior, but "write short
circuit" does not.
i.e. With locked session data, "write short circuit" would not change how
session manager works.I can't stress enough how confusing and potentially unsafe that is
It does not make difference since many users use session_commit()
to get more concurrency already. Simple documentation would be enough to
understand what it does.
There are freedom to shoot their own foot by themselves in any languages.
We have better performance/more complex operations/convenience in return.
Rather than discussing missing APIs are needed or not, I would like to
discuss
session_regenerate_id() issue. This is the design issue of session manager.
IMO.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
"write short circuit" omits "write" when $_SESSION hasn't change.
There
is
no point calling write API and writing to storage for the same data."write short circuit" as I understand it, is an exact copy of the
'lazy_write' option. This will be addressed in the previously
mentioned RFC that I'll post later today.No. It's not.
"lazy_write" does not lock session data while "write short circuit"
does.
In other words, "lazy_write" changes session behavior, but "write short
circuit" does not.
i.e. With locked session data, "write short circuit" would not change
how
session manager works.I can't stress enough how confusing and potentially unsafe that is
It does not make difference since many users use
session_commit()
to get more concurrency already. Simple documentation would be enough to
understand what it does.There are freedom to shoot their own foot by themselves in any languages.
We have better performance/more complex operations/convenience in return.
One more comment for this.
"write short cut" is designed to work with old(I mean existing) save
handlers.
Behavior does not change at all. New save handlers that support new save
handler API work much faster. That's it.
Save handlers like memcahe/memcached updates last access time stamp
by reading data. These save handler may omit writes at all when session data
hasn't changed. I don't think explanation is needed why this gives us better
performance.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
This is broken, consider the following (multiple tab/ajax/whatever
concurrency) scenario:Request1: session_start(['read_only' => TRUE]);
Request2:session_start(); unset($_SESSION['logged_in']);session_commit();
Request1: still logged in^ This screams "danger".
I think all of us know what it does.
It's also, redundant ... do you really believe you'd get +1 votes for
that feature if the voters were really aware that this would be just
an alias for (sesson_start() &&session_commit()) ?
It's not alias. It's much more efficient.
I don't think so. This is simply not what "read only" means which
makes this whole thing very misleading and if people who were supposed
to review the feature got confused by it, imagine what happens in
userland.
I might not explain well enough in the RFC, but most of use understood it's
a faster version of session_start();session_commit().
Calling session_commit() right after session_start() is common optimization
technique. Making it more efficient worth it.
I think there is no point to write back to the session data while
programmer only needs to read the data. Since
session_start();session_commit(); is optimization technique, making it more
efficient makes sense.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.netsession_start();session_commit()
This is broken, consider the following (multiple tab/ajax/whatever
concurrency) scenario:Request1: session_start(['read_only' => TRUE]);
Request2:session_start(); unset($_SESSION['logged_in']);
session_commit();
Request1: still logged in^ This screams "danger".
I think all of us know what it does.
It's also, redundant ... do you really believe you'd get +1 votes for
that feature if the voters were really aware that this would be just
an alias for (sesson_start() &&session_commit()) ?It's not alias. It's much more efficient.
I don't think so. This is simply not what "read only" means which
makes this whole thing very misleading and if people who were supposed
to review the feature got confused by it, imagine what happens in
userland.I might not explain well enough in the RFC, but most of use understood
it's a faster version ofsession_start();session_commit().Calling
session_commit()right aftersession_start()is common
optimization technique. Making it more efficient worth it.I think there is no point to write back to the session data while
programmer only needs to read the data. Since
session_start();session_commit(); is optimization technique, making it more
efficient makes sense.Regards,
Pressed send button before re-reading.
I think it could be understood what I mean.
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
The proper solution, would be to keep a timestamp stored with each
session. kind of a last-modified timestamp. (expiry does work off last
modification, right?). Anyhow, the trick is to check at session_start
whether that timestamp plus whatever session.expire is set at, already is
in the past - and when that happens, destroy the one session on storage and
act as if it hadn't been found.Such a timestamp already exists, you can't implement sessions without
one because there would be no way to know when a session have expired.
For the files handler, it'sfilemtime()(afaik) and the behavior that
you described is probably already in use (I see no reason not to).
Time stamp exists. It's correct partially.
The time stamp is not usable for HTTP session manager to manage session
properly. We may have new API that gets time stamp of session data, but it
would require large overhead. i.e. Need additional API call to get time
stamp = additional query to session storage. The overhead cannot be ignored.
The proper way to get and set the time stamp is "get/set time stamp" when
"store/retrieve session data". This way, there would not be overhead. User
may implement in their script, but it's a session manager task to begin
with.
However, that's not in the scope of GC.
Managing time stamp is the main task of session GC, isn't it?
a GC is only needed to clean up sessions that are never requested again
- and for that I fully agree that a separate - e.g. cron driven - "GC now"
job would be best (not affecting latency of running frontent requests too
much).^ That is what a garbage collector is, indeed. :)
On Thu, Mar 13, 2014 at 11:22 AM, Pierre Joye pierre.php@gmail.com
wrote:On Thu, Mar 13, 2014 at 8:45 AM, Yasuo Ohgaki yohgaki@ohgaki.net
wrote:It may be good idea to write a new save handler interface while keeping
older one.It would really helpful to target a later versions, maybe 6 and see
what we can do while minimizing the impact in userlands (whether or
not custom session handlers are implemented).But all the discussions and RFCs about basically the same needs have
been very confusing.What do you think? Is it something you could consider?
I am deffinately a proponent of that. I already mentioned earlier that
I have some ideas ... this is one of them.However, while Yasuo seems to always be keen on creating RFCs
immediately, I'd rather focus on the existing ones that are session
related:
You are the one who said there should be RFCs ;)
There are RFCs because I'm trying to change existing behaviors rather than
simple bug fixes.
Anyway, this was discussion of committed patch. It has much higher priority
than not implemented RFC, I suppose. I still think there should be GC
function. Programmers should control GC and there should be session
function for it rather than INI tweak.
https://wiki.php.net/rfc/secure-session-options-by-default
https://wiki.php.net/rfc/session_regenerate_id
5.6 is really close and this is important stuff that was supposed to
go in, but has not had updates recently and lagged behind.
Changes for future versions can wait a few days/weeks ...
It's better to write something so that it would be forgotten.
I forgot to update session_regenerate_id RFC since it became part of other
RFC. I'll update it soon.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
The proper solution, would be to keep a timestamp stored with each
session. kind of a last-modified timestamp. (expiry does work off last
modification, right?). Anyhow, the trick is to check at session_start
whether that timestamp plus whatever session.expire is set at, already is in
the past - and when that happens, destroy the one session on storage and act
as if it hadn't been found.Such a timestamp already exists, you can't implement sessions without
one because there would be no way to know when a session have expired.
For the files handler, it'sfilemtime()(afaik) and the behavior that
you described is probably already in use (I see no reason not to).Time stamp exists. It's correct partially.
The time stamp is not usable for HTTP session manager to manage session
properly. We may have new API that gets time stamp of session data, but it
would require large overhead. i.e. Need additional API call to get time
stamp = additional query to session storage. The overhead cannot be ignored.The proper way to get and set the time stamp is "get/set time stamp" when
"store/retrieve session data". This way, there would not be overhead. User
may implement in their script, but it's a session manager task to begin
with.However, that's not in the scope of GC.
Managing time stamp is the main task of session GC, isn't it?
Yasuo, you took 2 paragraphs that belong together and put them out of
context ...
However, while Yasuo seems to always be keen on creating RFCs
immediately, I'd rather focus on the existing ones that are session
related:You are the one who said there should be RFCs ;)
There are RFCs because I'm trying to change existing behaviors rather than
simple bug fixes.Anyway, this was discussion of committed patch. It has much higher priority
than not implemented RFC, I suppose. I still think there should be GC
function. Programmers should control GC and there should be session function
for it rather than INI tweak.
Exactly, focus on current tasks is what I'm saying.
In the timespan of 5 hours you asked me twice if I will write the RFC
and wrote it without even waiting for the reply.
Let's stop this nonsense now and discuss more important stuff. :)
Cheers,
Andrey.
Patrick Schaaf wrote:
This way there is no longer any correctness problem, and a GC is only
needed to clean up sessions that are never requested again - and for that I
fully agree that a separate - e.g. cron driven - "GC now" job would be best
(not affecting latency of running frontent requests too much).
I've not had much interest in this discussion as I am more than happy with how
things work now. It is not unusual for my 'client' sessions to last for an hour
or so without activity and their 'automatically' logging out was the problem
originally. So now I'm set never to clear session data and run a clear up on the
session directory before the site starts work in the morning. There are reasons
a system wide approach may be more practical than changing how a single session
handles things?
--
Lester Caine - G8HFL
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
Hi Andrey,
Are you going to write RFC for INI issue?
Not at this time, it won't make it into 5.6 anyway.
Julien Pauli already suggested that we should discuss that for PHP6
... I have some ideas, I'll start a discussion about them soon. :)
I think it good to document them now.
I don't mind at all adding E_DEPRECATED for session_module_name(),
mb_regex_encoding(), etc for PHP 5.7+.
Could you create the RFC?
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hi Andrey,
Are you going to write RFC for INI issue?
Not at this time, it won't make it into 5.6 anyway.
Julien Pauli already suggested that we should discuss that for PHP6
... I have some ideas, I'll start a discussion about them soon. :)I think it good to document them now.
I don't mind at all addingE_DEPRECATEDforsession_module_name(),
mb_regex_encoding(), etc for PHP 5.7+.Could you create the RFC?
I've created RFC
https://wiki.php.net/rfc/deprecate-ini-functions
Please feel free to update it.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
Hello,
I just saw this in the 5.6-alpha1 changelog:
- Implemented Request #54649 (Create session_serializer_name()). (Yasuo)
...- Implemented Request #11100 (session_gc() function). (Yasuo)
I didn't find an RFC and I don't know a way to search for a discussion
on internals (Google doesn't find one).session_serializer_name(): https://bugs.php.net/bug.php?id=54649
I don't think that this should've been implemented, especially with
the reasoning from the feature request ... The same could be applied
to all session ini options and especially now sincesession_start()
accepts them, it's useless IMO.
session_gc()was added in a similar fashion:
https://bugs.php.net/bug.php?id=11100There's already a comment about how the same thing can be achieved by
overloading SessionHandler::gc(), but a user could also just alter
session.gc_divisor, session.gc_probability to ensure that the garbage
collector is started.
And while session_serializer_name() is just redundant,session_gc()
could cause performance issues.I'd strongly suggest that these 2 functions should be removed before
PHP 5.6 is officially released.Regards,
Andrey Andreev.--
Hello guys,
When we branched 5.6, we branched it from master, with these commits into
it.
Ferenc did a hard job to spot what was in master at this time, and it looks
like he didn't notice those addings.
We should revert them.
We cannot accept new functions in 5.6 , if the community in its whole,
does not agree with them, by voting an RFC.
I'm sorry, but that is our policy, and we are not talking about "little"
functions we could blindly accept (because we are not bad dragons neither
:-p, some things are tiny and can be merged without heavy RFC process).
Those changes impact the session module, which is an important one.
Yasuo : I know you are actually working on RFCs for sessions for 5.6. I
would love to see them accepted and merged to 5.6. From what I know, one
has already been accepted partially (
https://wiki.php.net/rfc/session-lock-ini ). It seems like it's not been
merged yet ?
Is it because it cant, like you seem to say in
http://markmail.org/message/dcykcmq6exvkflf7 ?
Please, also remember that we are going to tag beta1 on 18th , your RFCs
need to be accepted and merged for this date.
Having said that, I agree we should clean the session API.
I agree session_module_name() is very uncommon and likely not very used,
and it should dissapear.
I agree as well that having to touch INI settings with ini_set() to change
the session behavior is not really nice, and that those should be feasable
using functions, or arguments to session_start();
However, all those points are BC breaking points, and can not be addressed
for 5.6 ; but I encourage all of you to start discussions about the session
module and those points, for PHP6.
Thank you all for your understanding, we all want to make PHP better, but
we all are humans :-p
Julien.Pauli