Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:67744
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.51 as permitted sender)
MIME-Version: 1.0
Sender: laruence@gmail.com
In-Reply-To: <CABBJUpcz3g6sWZR_K+p6fe0_nP7YZqW1XgZ54ktgA1KgD6xchg@mail.gmail.com>
References: <CAAyV7nEx32ni3J0dOQprJCXd=4D06XwS0-Ro811OtqmPeEyYJg@mail.gmail.com>
 <CABBJUpcz3g6sWZR_K+p6fe0_nP7YZqW1XgZ54ktgA1KgD6xchg@mail.gmail.com>
Date: Thu, 20 Jun 2013 18:16:08 +0800
Message-ID: <CABBJUpdJ97irYzC-u_GwugyNFBE_Zwh3A0m7C0srhiuCyk4=9g@mail.gmail.com>
To: Anthony Ferrara <ircmaxell@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Disabling the GC during shutdown
From: laruence@php.net (Laruence)

On Thu, Jun 20, 2013 at 6:12 PM, Laruence <laruence@php.net> wrote:
> On Thu, Jun 20, 2013 at 1:46 AM, Anthony Ferrara <ircmaxell@gmail.com> wrote:
>> All,
>>
>> We were discussing a range of bugs today with the garbage collector. For
>> example: https://bugs.php.net/bug.php?id=64827
>>
>> After quite a bit of digging, it appears what's happening is that the
>> garbage collector is running during the shutdown of PHP. So the destructors
>> are fired, and the variables are being destroyed when the GC run happens.
>> This means that the GC, while walking the variable tree runs into a
>> partially destructed object (where an entry of the hash table has already
>> been freed). This causes a segfault, and fun ensues.
> Hey:
>
>     Sorry,  but I don't this this explain is right.
>
>     if there is more than one refcount to a zval, then it should never be freed
>
>     and if a zval is freed, then it must also be removed from the gc roots.
>
>     according to your explain,  the gc segfault while walking through
> a hashtable of a object.
>
>     but that doesn't make any sense, since if it segfault in walking,
> then it should also segfault when trying to free the hash table later
> while dtor the object.
>
>     disable GC in shutdown is okey for me. but that is just try to
> cover the bug somewhere in the refcount handler.. not the right fix.
>
> thanks

And actually, I prefer not doing this(disable gc in shutdown) for now,
because maybe someday we can get a simple reproduce script(as we all
have see, it is very rare).

then the segfault will be fixed,  in the right way.

thanks
>
>
>>
>> Under normal conditions (not during shutdown), this does not appear to be
>> an issue, because the zval is destructed prior to the object destruction.
>> This means that there should never be a case where the GC hits a partially
>> freed object during normal execution.
>>
>> From what I can see, there are two possible fixes. The first would be to
>> change how object destruction works in the first place, to tie the variable
>> into the destruction process (basically refactor the object delref API to
>> also accept the current zval). That way the part of the code that makes the
>> decision to nuke the object can nuke the zval first (and hence prevent this
>> condition). However, this is a major API change and would effect a lot of
>> extensions that are using or tieing into this hook.
>>
>> The other option would be to simply disable the GC on shutdown. Considering
>> all of the variables are going to be thrown away anyway, having the GC run
>> during shutdown seems a bit wasteful anyway. So if we can kill two birds
>> with one stone, why not?
>>
>> I've prepared a basic patch:
>> https://github.com/ircmaxell/php-src/compare/gc_deactivate_on_shutdown
>>
>> I did confirm with odoucet (one of the original reporters) that this does
>> clear up his issue: https://gist.github.com/odoucet/5796378 (along with
>> trying a bunch of other things).
>>
>> There are a few out standing questions though:
>>
>> 1. Technically, all we need to do is force GC_G(gc_enabled) = 0 in
>> shutdown. But we could also use zend_alter_ini_entry which has the same
>> effect. The question comes is there any reason to go through the overhead
>> of altering the ini entry instead of the global directly? We do access the
>> global directly in other areas (but it's typically only set via ini)...
>>
>> 2. I put it in php_request_shutdown() after deactivate_ticks, but before it
>> calls shutdown functions. I could see it being moved after the shutdown
>> function call, but I'm not sure if it's worth it (either way). thoughts?
>>
>> 3. Can anyone think of a reason we'd want the GC enabled during the request
>> shutdown? I can't think of any...
>>
>> Additionally, considering that this does solve a segfault, is it worth
>> nominating this for 5.3? Or is it too risky (or something else I'm
>> missing)...
>>
>> Thanks,
>>
>> Anthony
>
>
>
> --
> Laruence  Xinchen Hui
> http://www.laruence.com/



--
Laruence  Xinchen Hui
http://www.laruence.com/