Re: [PHP-DEV] [RFC] Object oriented session handlers
On Sun, Jul 3, 2011 at 4:15 PM, Felipe Pena wrote:
> I like the idea (and voted +1 on it), but I've some consideration to do:
>
> +/* {{{ proto bool SessionHandler::open(string save_path, string session_name)
> + Wraps the old open handler */
> +PHP_METHOD(SessionHandler, open)
> +{
> + PS_SANITY_CHECK;
> +
> + PS(mod_user_is_open) = 1;
> + RETVAL_LONG(PS(default_mod)->s_open(&PS(mod_data), PS(save_path),
> PS(session_name) TSRMLS_CC));
> +}
> [..]
> +ZEND_BEGIN_ARG_INFO(arginfo_session_class_open, 0)
> + ZEND_ARG_INFO(0, save_path)
> + ZEND_ARG_INFO(0, session_name)
> +ZEND_END_ARG_INFO()
>
> This method does not take the save_path and session_name parameter, it
> just use the INI entry.
>
> This lead to following behavior:
> $x = new SessionHandler
> $x->open(1,2,3,4); // param is not used, and no param check at all
>
> It's missing void param check for the close method as well.
>
Hi,
Thanks for pointing that out. I've updated the patch with the param
checks and a couple of tests. Note that the close call still proceeds
if it gets arguments, although it raises the standard warning, because
the default handler would otherwise be left dangling.
The new patches (including 5.4 now):
http://spellign.com/patches/php-trunk-session-oo11.patch
http://spellign.com/patches/php-5.4-session-oo11.patch
http://spellign.com/patches/php-session-oo11-tests.patch
Cheers,
Arpad
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
Hi,
2011/6/2 Arpad Ray :
> Hi,
>
> A while ago I submitted a patch to allow session_set_save_handler() to
> accept a class, and support the inheritance of the default session
> handler's methods.
>
> The RFC has a more detailed description and the current patch:
> https://wiki.php.net/rfc/session-oo
>
> Changes since this was last discussed:
> - More sanity checking to prevent handlers being called in unexpected states
> - ZTS fixes
>
> Any thoughts?
>
> Regards,
>
> Arpad
I like the idea (and voted +1 on it), but I've some consideration to do:
+/* {{{ proto bool SessionHandler::open(string save_path, string session_name)
+ Wraps the old open handler */
+PHP_METHOD(SessionHandler, open)
+{
+ PS_SANITY_CHECK;
+
+ PS(mod_user_is_open) = 1;
+ RETVAL_LONG(PS(default_mod)->s_open(&PS(mod_data), PS(save_path),
PS(session_name) TSRMLS_CC));
+}
[..]
+ZEND_BEGIN_ARG_INFO(arginfo_session_class_open, 0)
+ ZEND_ARG_INFO(0, save_path)
+ ZEND_ARG_INFO(0, session_name)
+ZEND_END_ARG_INFO()
This method does not take the save_path and session_name parameter, it
just use the INI entry.
This lead to following behavior:
$x = new SessionHandler
$x->open(1,2,3,4); // param is not used, and no param check at all
It's missing void param check for the close method as well.
Thank you for helping us make PHP better. :)
--
Regards,
Felipe Pena
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
On 6/27/11 11:01 AM, Arpad Ray wrote: On Mon, Jun 27, 2011 at 5:00 PM, Arpad Ray wrote: On Mon, Jun 27, 2011 at 4:52 AM, Larry Garfield wrote: I'm a bit confused. If the session handler goes out of its way to ensure that it's the last thing to run, wouldn't that cause an issue if it tries to write session data after, say, the database connection object it wants to use has already been cleaned up? Or is that the use case for the "do not register" parameter? It seems like it would be a fairly common use case... If the database object isn't manually cleaned up (all references to it are unset) then that will take place in the second stage of the request shutdown process, whereas the first stage is running the shutdown functions, so that shouldn't be an issue. The most common use case for the second argument is where the user already calls session_write_close() (probably in their own shutdown function) and doesn't reopen it - it's then useless for us to register and call our shutdown function. Regards, Arpad Ah, so the order at the end of the request would be: - Fire normal registered shutdown routines. - Fire session-close shutdown routine. - Call destructors on all objects still outstanding. That seems fine to me. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
On Mon, Jun 27, 2011 at 5:00 PM, Arpad Ray wrote: > On Mon, Jun 27, 2011 at 4:52 AM, Larry Garfield > wrote: >> I'm a bit confused. If the session handler goes out of its way to ensure >> that it's the last thing to run, wouldn't that cause an issue if it tries to >> write session data after, say, the database connection object it wants to >> use has already been cleaned up? Or is that the use case for the "do not >> register" parameter? It seems like it would be a fairly common use case... >> > > If the database object isn't manually cleaned up (all references to it > are unset) then that will take place in the second stage of the > request shutdown process, whereas the first stage is running the > shutdown functions, so that shouldn't be an issue. > > The most common use case for the second argument is where the user > already calls session_write_close() (probably in their own shutdown > function) and doesn't reopen it - it's then useless for us to register > and call our shutdown function. > > Regards, > > Arpad > Oops, CCing the list again. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
On 06/25/2011 12:13 AM, Arpad Ray wrote: Hi, I've updated the patches again. The most significant change is that the shutdown function registers another shutdown function when it's called, to (almost) ensure that it's always the last one, and therefore user shutdown functions should always find the session available as expected. I'd especially appreciate feedback or improvements on this because it feels very hacky. One option is to add a separate hash of internal shutdown functions which run after the user shutdown functions, another is to add an element to the module entry, a la RSHUTDOWN, which gets run in between steps 1 and 2 in the shutdown process (see main.c:1632). I don't know what other uses there are for this, so I've stuck with the double shutdown function method in this patch. I'd also like to avoid complicating the vote on this patch with a tangential issue. I've added a second optional argument to session_set_save_handler($obj) which is true by default and indicates whether the shutdown function should be registered - this is so that users with their own manual shutdown procedure can disable the automatic approach. I think that most users will take advantage of the automatically registered shutdown function, but this lets those with a custom shutdown procedure handle the session shutdown within the lifetime of the script and save the slight overhead. http://spellign.com/patches/php-trunk-session-oo10.patch http://spellign.com/patches/php-trunk-session-oo10-tests.patch Regards, Arpad I'm a bit confused. If the session handler goes out of its way to ensure that it's the last thing to run, wouldn't that cause an issue if it tries to write session data after, say, the database connection object it wants to use has already been cleaned up? Or is that the use case for the "do not register" parameter? It seems like it would be a fairly common use case... --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
On Sat, Jun 25, 2011 at 6:13 AM, Arpad Ray wrote: > The most significant change is that the shutdown function registers > another shutdown function when it's called, to (almost) ensure that > it's always the last one, and therefore user shutdown functions should > always find the session available as expected. I'd especially > appreciate feedback or improvements on this because it feels very > hacky. One option is to add a separate hash of internal shutdown > functions which run after the user shutdown functions, another is to > add an element to the module entry, a la RSHUTDOWN, which gets run in > between steps 1 and 2 in the shutdown process (see main.c:1632). > To clarify the "almost" - it's still possible for a user shutdown function registered after session_set_save_handler() is called, to register another shutdown function, which would be run after the 2nd session shutdown function. This is one reason why I prefer the other methods, but it's a pretty small risk. Regards, Arpad -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
Hi, I've updated the patches again. The most significant change is that the shutdown function registers another shutdown function when it's called, to (almost) ensure that it's always the last one, and therefore user shutdown functions should always find the session available as expected. I'd especially appreciate feedback or improvements on this because it feels very hacky. One option is to add a separate hash of internal shutdown functions which run after the user shutdown functions, another is to add an element to the module entry, a la RSHUTDOWN, which gets run in between steps 1 and 2 in the shutdown process (see main.c:1632). I don't know what other uses there are for this, so I've stuck with the double shutdown function method in this patch. I'd also like to avoid complicating the vote on this patch with a tangential issue. I've added a second optional argument to session_set_save_handler($obj) which is true by default and indicates whether the shutdown function should be registered - this is so that users with their own manual shutdown procedure can disable the automatic approach. I think that most users will take advantage of the automatically registered shutdown function, but this lets those with a custom shutdown procedure handle the session shutdown within the lifetime of the script and save the slight overhead. http://spellign.com/patches/php-trunk-session-oo10.patch http://spellign.com/patches/php-trunk-session-oo10-tests.patch Regards, Arpad -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
On Mon, Jun 20, 2011 at 10:37 AM, Richard Quadling wrote:
> On 20 June 2011 01:39, Arpad Ray wrote:
>> On Mon, Jun 6, 2011 at 5:31 PM, Richard Quadling wrote:
>>> Not an internals expert, but I do have a question.
>>>
>>> When would the session handler object be destroyed?
>>>
>>> If sessions are being logged to a DB (maybe via a userland PHP class),
>>> is internal reference counting enough to handle the order of
>>> destruction?
>>>
>>> If the DB connection is destroyed before the session handler gets it
>>> destructor called (or the session_write_close equivalent), the session
>>> won't be able to log the data and there would be no warning to the
>>> client as engine is in auto tidy up mode.
>>>
>>
>> Hi,
>>
>> Many thanks for your question, because I hadn't given the matter much
>> thought. The current patch (#7) uses static methods so it doesn't
>> change the status quo - the user would need still need to manually
>> close the session (or register a shutdown function) in order to use an
>> object in the close() method.
>>
>> I've spent some time thinking about this and I think we have a couple
>> of options. First of all I've changed the interface so that the
>> methods are no longer static, it would already accept an object before
>> but it would just use it find the class name, and call the methods
>> statically. Now it only accepts an object.
>>
>> The two options I've implemented are:
>>
>> Destructor in the SessionHandler class:
>> http://spellign.com/patches/php-trunk-session-oo8-destruct.patch
>>
>> This works ok with some provisions:
>> - The user's SessionHandler class must hold on to any objects it will
>> need in close(), (in a property, presumably). While it's possible that
>> the session handler would still get destructed afterwards, this is the
>> only way to ensure it.
>> - If the user overrides __destruct(), they must call parent::__destruct().
>>
>> Automatically register a shutdown function:
>> http://spellign.com/patches/php-trunk-session-oo8-shutdown_function.patch
>>
>> The only argument I can think of against it is that it's possible the
>> user might, out of habit, register their own shutdown function *after*
>> calling session_set_save_handler(). In that case their shutdown
>> function would find the session already closed.
>>
>>> Obviously, if the developer takes care and calls the destructors in
>>> the right order, then everything will be OK, but this is not something
>>> I see very often.
>>>
>>> What about circular references and interdependent references?
>>>
>>
>> This would give the destructor option some trouble - in this case
>> instances are destructed in the order in which they were created, and
>> it seems likely that the DB object from your example would be created
>> first.
>>
>> I prefer the shutdown function option. We could even ensure that the
>> user's shutdown function always gets called first by adding a separate
>> hash of internal shutdown functions, however I think that would
>> overcomplicate matters for something we can clearly document.
>>
>> The option I haven't mentioned is to preserve the status quo, I think
>> that would be a pity since we have the chance to address it while
>> keeping BC now.
>>
>> I've moved the tests into a separate patch so the above differences are
>> clearer:
>> http://spellign.com/patches/php-trunk-session-oo8-tests.patch
>>
>> Regards,
>>
>> Arpad
>>
>
> Thanks for looking into this a lot more.
>
> I've always used a First-In-Last-Out (FILO) creation/destruction order
> and incorporated the concept of "ownership".
>
> Any time an instance of a class is created, it has to be "owned" by
> another instance (of any type) or by a pseudo top-parent. In my case,
> I always have an instance of tApp, to which I attach my DB resource
> container (invariable I communicate with multiple unlink DB servers)
> and a session class (old skool).
>
> tApp is created, DB(s) are linked to. Session is started.
>
> As part of the app destructor, the session is closed first. Always.
> Then the DB connectors. For me, during the shutdown of tApp, there is
> no more activity to be processed, so the cleanup can take place in a
> controlled manner. And the reverse order seems the best way.
>
> And I always call the destructor on tApp as part of my code. A
> try{}finally{} clause would certainly be beneficial but probably not
> essential.
>
> Hopefully, by the time my script has finished, all resources are
> closed and finalised and the script engine shutdown/cleanup cycle is
> doing nothing.
>
>
> Essentially, it is moving the cleanup process into the hands of the
> developer. If they don't control the order, I'm not sure you can
> predict the order. Or if it can be predicted, it may not be the
> desired order.
>
>
>
> I think this is a great addition to PHP. I think it needs to be
> PHP\SessionHandler. I think all new classes should be namespaced to
> PHP, but that's another topic I'm sure.
>
>
> --
> Richard Quadling
> Twitter : EE : Zend : PHP
Re: [PHP-DEV] [RFC] Object oriented session handlers
On 20 June 2011 01:39, Arpad Ray wrote:
> On Mon, Jun 6, 2011 at 5:31 PM, Richard Quadling wrote:
>> Not an internals expert, but I do have a question.
>>
>> When would the session handler object be destroyed?
>>
>> If sessions are being logged to a DB (maybe via a userland PHP class),
>> is internal reference counting enough to handle the order of
>> destruction?
>>
>> If the DB connection is destroyed before the session handler gets it
>> destructor called (or the session_write_close equivalent), the session
>> won't be able to log the data and there would be no warning to the
>> client as engine is in auto tidy up mode.
>>
>
> Hi,
>
> Many thanks for your question, because I hadn't given the matter much
> thought. The current patch (#7) uses static methods so it doesn't
> change the status quo - the user would need still need to manually
> close the session (or register a shutdown function) in order to use an
> object in the close() method.
>
> I've spent some time thinking about this and I think we have a couple
> of options. First of all I've changed the interface so that the
> methods are no longer static, it would already accept an object before
> but it would just use it find the class name, and call the methods
> statically. Now it only accepts an object.
>
> The two options I've implemented are:
>
> Destructor in the SessionHandler class:
> http://spellign.com/patches/php-trunk-session-oo8-destruct.patch
>
> This works ok with some provisions:
> - The user's SessionHandler class must hold on to any objects it will
> need in close(), (in a property, presumably). While it's possible that
> the session handler would still get destructed afterwards, this is the
> only way to ensure it.
> - If the user overrides __destruct(), they must call parent::__destruct().
>
> Automatically register a shutdown function:
> http://spellign.com/patches/php-trunk-session-oo8-shutdown_function.patch
>
> The only argument I can think of against it is that it's possible the
> user might, out of habit, register their own shutdown function *after*
> calling session_set_save_handler(). In that case their shutdown
> function would find the session already closed.
>
>> Obviously, if the developer takes care and calls the destructors in
>> the right order, then everything will be OK, but this is not something
>> I see very often.
>>
>> What about circular references and interdependent references?
>>
>
> This would give the destructor option some trouble - in this case
> instances are destructed in the order in which they were created, and
> it seems likely that the DB object from your example would be created
> first.
>
> I prefer the shutdown function option. We could even ensure that the
> user's shutdown function always gets called first by adding a separate
> hash of internal shutdown functions, however I think that would
> overcomplicate matters for something we can clearly document.
>
> The option I haven't mentioned is to preserve the status quo, I think
> that would be a pity since we have the chance to address it while
> keeping BC now.
>
> I've moved the tests into a separate patch so the above differences are
> clearer:
> http://spellign.com/patches/php-trunk-session-oo8-tests.patch
>
> Regards,
>
> Arpad
>
Thanks for looking into this a lot more.
I've always used a First-In-Last-Out (FILO) creation/destruction order
and incorporated the concept of "ownership".
Any time an instance of a class is created, it has to be "owned" by
another instance (of any type) or by a pseudo top-parent. In my case,
I always have an instance of tApp, to which I attach my DB resource
container (invariable I communicate with multiple unlink DB servers)
and a session class (old skool).
tApp is created, DB(s) are linked to. Session is started.
As part of the app destructor, the session is closed first. Always.
Then the DB connectors. For me, during the shutdown of tApp, there is
no more activity to be processed, so the cleanup can take place in a
controlled manner. And the reverse order seems the best way.
And I always call the destructor on tApp as part of my code. A
try{}finally{} clause would certainly be beneficial but probably not
essential.
Hopefully, by the time my script has finished, all resources are
closed and finalised and the script engine shutdown/cleanup cycle is
doing nothing.
Essentially, it is moving the cleanup process into the hands of the
developer. If they don't control the order, I'm not sure you can
predict the order. Or if it can be predicted, it may not be the
desired order.
I think this is a great addition to PHP. I think it needs to be
PHP\SessionHandler. I think all new classes should be namespaced to
PHP, but that's another topic I'm sure.
--
Richard Quadling
Twitter : EE : Zend : PHPDoc
@RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY : bit.ly/lFnVea
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
On Mon, Jun 6, 2011 at 5:31 PM, Richard Quadling wrote: > Not an internals expert, but I do have a question. > > When would the session handler object be destroyed? > > If sessions are being logged to a DB (maybe via a userland PHP class), > is internal reference counting enough to handle the order of > destruction? > > If the DB connection is destroyed before the session handler gets it > destructor called (or the session_write_close equivalent), the session > won't be able to log the data and there would be no warning to the > client as engine is in auto tidy up mode. > Hi, Many thanks for your question, because I hadn't given the matter much thought. The current patch (#7) uses static methods so it doesn't change the status quo - the user would need still need to manually close the session (or register a shutdown function) in order to use an object in the close() method. I've spent some time thinking about this and I think we have a couple of options. First of all I've changed the interface so that the methods are no longer static, it would already accept an object before but it would just use it find the class name, and call the methods statically. Now it only accepts an object. The two options I've implemented are: Destructor in the SessionHandler class: http://spellign.com/patches/php-trunk-session-oo8-destruct.patch This works ok with some provisions: - The user's SessionHandler class must hold on to any objects it will need in close(), (in a property, presumably). While it's possible that the session handler would still get destructed afterwards, this is the only way to ensure it. - If the user overrides __destruct(), they must call parent::__destruct(). Automatically register a shutdown function: http://spellign.com/patches/php-trunk-session-oo8-shutdown_function.patch The only argument I can think of against it is that it's possible the user might, out of habit, register their own shutdown function *after* calling session_set_save_handler(). In that case their shutdown function would find the session already closed. > Obviously, if the developer takes care and calls the destructors in > the right order, then everything will be OK, but this is not something > I see very often. > > What about circular references and interdependent references? > This would give the destructor option some trouble - in this case instances are destructed in the order in which they were created, and it seems likely that the DB object from your example would be created first. I prefer the shutdown function option. We could even ensure that the user's shutdown function always gets called first by adding a separate hash of internal shutdown functions, however I think that would overcomplicate matters for something we can clearly document. The option I haven't mentioned is to preserve the status quo, I think that would be a pity since we have the chance to address it while keeping BC now. I've moved the tests into a separate patch so the above differences are clearer: http://spellign.com/patches/php-trunk-session-oo8-tests.patch Regards, Arpad -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Object oriented session handlers
Forgot to keep the list on this one. On Mon, Jun 6, 2011 at 11:19 AM, Mike Willbanks wrote: > A while ago I submitted a patch to allow session_set_save_handler() to >> accept a class, and support the inheritance of the default session >> handler's methods. >> >> The RFC has a more detailed description and the current patch: >> https://wiki.php.net/rfc/session-oo >> >> Changes since this was last discussed: >> - More sanity checking to prevent handlers being called in unexpected >> states >> - ZTS fixes >> >> Any thoughts? >> > > Unfortunately the class may cause a BC break due to the naming. Although > namespaces could help in reducing the potential scope of a BC break. > > This is a huge win for those of us with generally highly custom session > handlers. This has been a consistent point of pain and being able to > "extend" rather than reimplement would be a very nice enhancement. > > Now on the new SessionHandler object; is this now utilized by default? I > seen the wrapper in the patch but it seems like we utilize all of the > existing code with modifications to also utilize an object? Should > the procedural way be deprecated at some point or is the plan to support > both styles? > > Regards, > > Mike > >
Re: [PHP-DEV] [RFC] Object oriented session handlers
On 3 June 2011 02:18, Arpad Ray wrote: > Hi, > > A while ago I submitted a patch to allow session_set_save_handler() to > accept a class, and support the inheritance of the default session > handler's methods. > > The RFC has a more detailed description and the current patch: > https://wiki.php.net/rfc/session-oo > > Changes since this was last discussed: > - More sanity checking to prevent handlers being called in unexpected states > - ZTS fixes > > Any thoughts? > > Regards, > > Arpad Not an internals expert, but I do have a question. When would the session handler object be destroyed? If sessions are being logged to a DB (maybe via a userland PHP class), is internal reference counting enough to handle the order of destruction? If the DB connection is destroyed before the session handler gets it destructor called (or the session_write_close equivalent), the session won't be able to log the data and there would be no warning to the client as engine is in auto tidy up mode. Obviously, if the developer takes care and calls the destructors in the right order, then everything will be OK, but this is not something I see very often. What about circular references and interdependent references? -- Richard Quadling Twitter : EE : Zend : PHPDoc @RQuadling : e-e.com/M_248814.html : bit.ly/9O8vFY : bit.ly/lFnVea -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [RFC] Object oriented session handlers
Hi, A while ago I submitted a patch to allow session_set_save_handler() to accept a class, and support the inheritance of the default session handler's methods. The RFC has a more detailed description and the current patch: https://wiki.php.net/rfc/session-oo Changes since this was last discussed: - More sanity checking to prevent handlers being called in unexpected states - ZTS fixes Any thoughts? Regards, Arpad -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
