Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:67735
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.173 as permitted sender)
MIME-Version: 1.0
Date: Wed, 19 Jun 2013 13:46:18 -0400
Message-ID: <CAAyV7nEx32ni3J0dOQprJCXd=4D06XwS0-Ro811OtqmPeEyYJg@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e013a28602e25a804df856820
Subject: Disabling the GC during shutdown
From: ircmaxell@gmail.com (Anthony Ferrara)

--089e013a28602e25a804df856820
Content-Type: text/plain; charset=ISO-8859-1

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.

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

--089e013a28602e25a804df856820--