Bug #78379 [ReO]: Cast to object confuses GC, causes crash

From: Date: Fri, 09 Aug 2019 03:33:38 +0000
Subject: Bug #78379 [ReO]: Cast to object confuses GC, causes crash
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-222148@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=78379&edit=1 ID: 78379 Updated by: tstarling@php.net Reported by: tstarling@php.net Summary: Cast to object confuses GC, causes crash Status: Re-Opened Type: Bug Package: Reproducible crash Operating System: Linux PHP Version: 7.2Git-2019-08-06 (Git) Block user comment: N Private report: N New Comment: I guess it's good enough. The trace output is certainly better than it was. I did start work on a patch which tries to make property tables be children of objects. The problem I ran into is making sure property tables are not directly destroyed by the garbage collector, since zend_object_std_dtor() etc. doesn't check if the property array has already been destroyed before destroying it, leading to an assertion failure. Maybe it is a thing to try against master rather than in a bugfix for all branches. Previous Comments: ------------------------------------------------------------------------ [2019-08-08 14:18:44] dmitry@php.net Proposed patches https://gist.github.com/dstogov/43a992d481f65ac16c454e1a292be38e seem to fix the second problem ------------------------------------------------------------------------ [2019-08-08 08:03:25] dmitry@php.net The committed patch https://github.com/php/php-src/commit/358379be22c4e20f4942737e0e90422977355c63 doesn't fix the second problem. ------------------------------------------------------------------------ [2019-08-08 07:09:12] dmitry@php.net Automatic comment on behalf of dmitry@zend.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=358379be22c4e20f4942737e0e90422977355c63 Log: Fixed bug #78379 (Cast to object confuses GC, causes crash) ------------------------------------------------------------------------ [2019-08-08 03:13:37] tstarling@php.net Your patch prevents the array being freed, but it doesn't stop its reference count from going negative, since gc_mark_grey() is still the same, and it won't collect cycles that include the shared array as part of the loop. Both issues can be seen in this test case: class E {} function f() { $e1 = new E; $e2 = new E; $a = ['e2' => $e2]; $e1->a = (object)$a; $e2->e1 = $e1; $e2->a = (object)$a; } f(); gc_collect_cycles(); echo "End\n"; With ZEND_GC_DEBUG=2 you can see it report rc=-1 for object(E)#2, then it says there is "Nothing to free". It's just a memory leak rather than a crash, but I'm considering doing a patch of my own along the lines of the third paragraph of the bug description, so we can see if that looks better. ------------------------------------------------------------------------ [2019-08-07 11:36:23] dmitry@php.net This patch seems to fix the problem, but it may be incomplete. https://gist.github.com/dstogov/43a992d481f65ac16c454e1a292be38e ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=78379 -- Edit this bug report at https://bugs.php.net/bug.php?id=78379&edit=1

« previous php.bugs (#222148) next »