Re: Disabling the GC during shutdown

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/