Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:51713
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.214.170 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:sender:in-reply-to:references:date
         :x-google-sender-auth:message-id:subject:from:to:cc:content-type;
        b=EjQp81zGozt6W4Lvv9yqIbTOW9jeTsvtF3pxmhQpRfeGxc6SQd4Irvcn7Oqcyqnm3z
         g+l+ZYfBACARWRe3AOsx/1Sboym0WmoTKbiToTlhVq8o8iqeSDnFnMPepd4nOrTdJvPD
         AvBAyVNQljjzXf3MW8pSO4pF+OevkjP25dcH0=
MIME-Version: 1.0
Sender: tyra3l@gmail.com
In-Reply-To: <4D803F21.1010209@wikimedia.org>
References: <4D803F21.1010209@wikimedia.org>
Date: Wed, 16 Mar 2011 09:26:55 +0100
Message-ID: <AANLkTimd21XtHB2=f_HYCazq_v4hLqM8q5zUpS5_eOxW@mail.gmail.com>
To: Tim Starling <tstarling@wikimedia.org>
Cc: internals Mailing List <internals@lists.php.net>, Dmitry Stogov <dmitry@zend.com>, 
	=?UTF-8?Q?Johannes_Schl=C3=BCter?= <johannes@php.net>
Content-Type: multipart/alternative; boundary=20cf3005da88c91c94049e954ed9
Subject: Re: [PHP-DEV] [patch] Session RSHUTDOWN, shutdown_destructors() issues
From: info@tyrael.hu (Ferenc Kovacs)

--20cf3005da88c91c94049e954ed9
Content-Type: text/plain; charset=UTF-8

2011/3/16 Tim Starling <tstarling@wikimedia.org>

> This is a followup to http://bugs.php.net/bug.php?id=54157 . Johannes
> said I should post here.
>
> In the course of my MediaWiki development work, I hit a strange issue
> involving session_set_save_handler().
>
> PHP's built-in session handler is pretty much useless when you have
> more than one server serving a given site, so
> session_set_save_handler() is essential. MediaWiki has a feature
> allowing it to save sessions via its memcached client. A suitable
> client object is put in $wgMemc during setup, which is used when the
> session is closed. A minor change to the way this variable was
> initialised caused it to disappear from the global symbol table before
> the session save handler was called.
>
> This turned out to be due to a broken algorithm in
> shutdown_destructors() in zend_execute_API.c. The algorithm basically
> does this:
>
> 1. Delete all objects from the global symbol table which have a zval
> reference count of 1
> 2. If any objects were deleted, go to step 1.
>
> Obviously the intention is to first delete the references, and then to
> delete the objects which were referred to. It doesn't work that way:
> if you have a reference, both symbol table entries point to the same
> zval, so they both have a reference count of 2, so the algorithm skips
> both and leaves them intact for RSHUTDOWN. My $wgMemc initialisation
> change caused the reference count to drop from 2 to 1, so the variable
> was deleted.
>
> Fixing this function to correctly delete all objects from the global
> symbol table would be easy enough (against 5.3):
>
> http://tstarling.com/stuff/shutdown_destructors-sanity.patch
>
> But please don't apply that, because it would break all released
> versions of MediaWiki.
>
> We now come to the next strange thing about this code: why is it
> attempting to delete all objects from the global symbol table anyway?
> There are plenty of other ways to store objects: function static
> variables, class static variables, etc. Deleting them from $GLOBALS
> doesn't stop them from being accessed. And besides, we know that
> there's no problem with accessing an object after __destruct() has
> been called on it, because that was the whole point of splitting out
> shutdown_destructors() in the first place, as a comment in
> shutdown_executor() explains:
>
> /* Removed because this can not be safely done, e.g. in this situation:
>   Object 1 creates object 2
>   Object 3 holds reference to object 2.
>   Now when 1 and 2 are destroyed, 3 can still access 2 in its
> destructor, with
>   very problematic results */
> /*         zend_objects_store_call_destructors(&EG(objects_store)
> TSRMLS_CC); */
>
> Just removing the whole loop would suit me, as a 5.3.x stopgap measure:
>
> http://tstarling.com/stuff/no-global-deletion.patch
>
> But that still leaves the problem that the session save handler is
> called in the strange and scary world of half-shut-down PHP. We don't
> know what RSHUTDOWN functions have been called before the session
> RSHUTDOWN, so we don't know what userspace code will work and what
> won't. A workaround is easy enough:
>
> http://www.mediawiki.org/wiki/Special:Code/MediaWiki/83147
>
> but I thought I would be a good open source citizen and come up with a
> proper solution, against trunk. It is attached and at:
>
> http://tstarling.com/stuff/session-pre-deactivate.patch
>
> The idea is to introduce a pre-RSHUTDOWN hook, allowing modules to
> call user code in a relatively sane environment. It's implemented in a
> way that is closely analogous to the post-deactivate hook. I had
> trouble testing it because trunk was so broken that it wouldn't run
> MediaWiki, but the patch appears to work for simple CLI test cases.
>
> -- Tim Starling
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

Hi.

Could you please also open a ticket in the bug tracker?
thanks

Tyrael

--20cf3005da88c91c94049e954ed9--