Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:53454
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.170 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :cc:content-type;
        b=TqNvn3BkIGyaGBXerOW3FLGDUYldi1LodL16I/0gsK8z7jKNhxKnWgwr8bMk23jRRX
         VLMInK+RtexZ3MSwE/mfqmWh2m7482YJZ82gnFyAAFq8m030vO7h6sHcS5lNbGbcOgVY
         6DXRXD4VnLIxyUDswDVKxjriAdIdk3F6Du2QA=
MIME-Version: 1.0
In-Reply-To: <BANLkTik6KaHhY+RvTYY5+DEphsGWrekPrA@mail.gmail.com>
References: <BANLkTi=Kbbv_Vt9zb+e3faoUqUWekDr8Gw@mail.gmail.com>
	<BANLkTikWE4xdJs7UnX3ZfcyVO3KrxP+jLg@mail.gmail.com>
	<BANLkTinUPizFtNR2SeiCtsbwyVB2SUQUHQ@mail.gmail.com>
	<BANLkTik6KaHhY+RvTYY5+DEphsGWrekPrA@mail.gmail.com>
Date: Mon, 20 Jun 2011 23:41:47 +0100
Message-ID: <BANLkTi=PE_HVbbv6NtjAK=pCdtAdkTXTyw@mail.gmail.com>
To: RQuadling@gmail.com
Cc: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Subject: Re: [PHP-DEV] [RFC] Object oriented session handlers
From: arraypad@gmail.com (Arpad Ray)

On Mon, Jun 20, 2011 at 10:37 AM, Richard Quadling <rquadling@gmail.com> wrote:
> On 20 June 2011 01:39, Arpad Ray <arraypad@gmail.com> wrote:
>> On Mon, Jun 6, 2011 at 5:31 PM, Richard Quadling <rquadling@gmail.com> 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
>

Hi,

I've updated the patches with some changes suggested by Derick:

* Added PHPAPI in a couple of places so it doesn't break on Windows
* Changed PSF to PHP_SESSION_FUNC to avoid possible conflicts
* Split the tests into separate files

I'm also only including the shutdown function option, since I think
the destructor option is so clearly inferior for the reasons outlined
in my previous email that it's not worth complicating the vote.

The new patches are at:
http://spellign.com/patches/php-trunk-session-oo9.patch
http://spellign.com/patches/php-trunk-session-oo9-tests.patch

Regards,

Arpad