Bug #65387 [ReO]: Circular references in SPL iterators are not garbage collected

From: Date: Fri, 02 Oct 2020 09:14:40 +0000
Subject: Bug #65387 [ReO]: Circular references in SPL iterators are not garbage collected
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-229333@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=65387&edit=1

 ID:                 65387
 Updated by:         nikic@php.net
 Reported by:        bugs dot php dot net at ss dot chernousov dot net
 Summary:            Circular references in SPL iterators are not garbage
                     collected
 Status:             Re-Opened
 Type:               Bug
 Package:            Scripting Engine problem
 Operating System:   Any
 PHP Version:        5.5.1
 Assigned To:        nikic
 Block user comment: N
 Private report:     N

 New Comment:

I've implemented a fix for test case 8 in https://github.com/php/php-src/pull/6262.
However, this one will only go into PHP 8.1, because it changes the internal PDO ABI.


Previous Comments:
------------------------------------------------------------------------
[2020-10-02 08:37:17] nikic@php.net

I checked test case 7, and that one no longer leaks since PHP 8.0. GC support for  CurlHandle has
been implemented as part of the migration from resources to objects (resources do not support cycle
GC).

------------------------------------------------------------------------
[2020-10-01 15:23:56] php dot net at ss dot st dot tc

Thanks, @nikic, I appreciate your attention to this problem.

In the meantime, correction my previous comment, test 8 should read as follows (replaced $db with
$obj->a):

// TEST 8: PDO/sqlite callback
$obj->a = new PDO('sqlite::memory:');
$obj->a->sqliteCreateFunction('md5rev', static function() use ($obj) {}, 1);

------------------------------------------------------------------------
[2020-10-01 15:19:53] nikic@php.net

This issue isn't really related to callbacks, it's a question of whether the specific
object holding the callback implements GC support. As such, it needs to be addressed on a
case-by-case basis. Most of the examples you listed were in 3rd-party extensions over which we have
no control.

I will however check the additional two cases you just provided, those are part of our
responsibility :)

------------------------------------------------------------------------
[2020-10-01 15:14:32] php dot net at ss dot st dot tc

The bug author here.

Thank you for fixing the issue with SPL iterators. However, the original issue was about circular
references on callbacks in general, and not specifically about handling circular references in SPL
iterators. I see the bug title was changed on 2020-10-01, affecting its meaning quite much. Among
the 6 cases provided (https://gist.github.com/5lava/53aa2e53c7f8c658f045), only one of them had to
do with SPL iterators. I tested that case and I confirm it's fixed, but the 2nd one still
leaks, as it did 7 years ago.

I did not re-test cases 3-6 since then, they are rather specific and I don't know if
that's something that has to be addressed in the corresponding extensions or PHP core itself.

That does not, however, diminish the scale of the problem. For example, with curl:

// TEST 7: curl callback
$obj->a = curl_init('https://www.php.net/');
curl_setopt($obj->a, CURLOPT_HEADERFUNCTION, static function() use ($obj) {});

That leaks. Or with PDO/sqlite:

// TEST 8: PDO/sqlite callback
$db = new PDO('sqlite::memory:');
$db->sqliteCreateFunction('md5rev', static function() use ($obj) {}, 1);

That leaks, too. My point being: the bug (the reported one!) was NOT fixed.

@nikic I'd appreciate some input from you regarding this matter. Thank you.

------------------------------------------------------------------------
[2020-10-01 14:13:22] nikic@php.net

Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=afab9eb48c883766b7870f76f2e2b0a4bd575786
Log: Fix bug #65387

------------------------------------------------------------------------


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=65387


--
Edit this bug report at https://bugs.php.net/bug.php?id=65387&edit=1


Thread (11 messages)

« previous php.bugs (#229333) next »