php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #65387 Circular references in SPL iterators are not garbage collected
Submitted: 2013-08-04 20:14 UTC Modified: 2020-10-12 11:03 UTC
Votes:4
Avg. Score:5.0 ± 0.0
Reproduced:4 of 4 (100.0%)
Same Version:2 (50.0%)
Same OS:1 (25.0%)
From: bugs dot php dot net at ss dot chernousov dot net Assigned: nikic (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: 5.5.1 OS: Any
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: bugs dot php dot net at ss dot chernousov dot net
New email:
PHP Version: OS:

 

 [2013-08-04 20:14 UTC] bugs dot php dot net at ss dot chernousov dot net
Description:
------------
GC fails to resolve the circular reference if object A retains a reference to a 
callback in object B and object B retains a reference to object A. Both objects 
leak.

Native PHP stuff like SPL iterators with callbacks and Stream callbacks are also 
vulnerable to this problem.

This does not apply to userland PHP objects (i.e. objects of classes that were 
defined in PHP scripts by a user).

I provided a test script with a number of tests, including SPL iterators with 
callbacks, Stream callbacks, as well as 3rd-party extensions like pecl-event, 
pecl-ev, pecl-libevent, pecl-eio.

Test script:
---------------
https://gist.github.com/5lava/53aa2e53c7f8c658f045

Expected result:
----------------
==== NULL ====
==== GC ====
Obj::__destruct
==== THE END ====

or

==== NULL ====
Obj::__destruct
==== GC ====
==== THE END ====


Actual result:
--------------
==== NULL ====
==== GC ====
==== THE END ====
Obj::__destruct


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-03-26 23:40 UTC] nikic@php.net
-Status: Open +Status: Verified
 [2016-03-26 23:40 UTC] nikic@php.net
Still leaks, even in PHP 7. dual_it doesn't implement GC handling.
 [2020-09-24 09:09 UTC] mvorisek at mvorisek dot cz
@nikic why dual_it can not be GCed like any other object and is this something than can be fixed?

https://3v4l.org/AL2Hr the issue seems to be present even if the callback is static but the (outer) Iterator is linked with any object
 [2020-10-01 12:46 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2020-10-01 13:47 UTC] nikic@php.net
-Summary: Memleak:Circular references on callbacks can't be resolved by Garbage Collector +Summary: Circular references in SPL iterators are not garbage collected
 [2020-10-01 14:13 UTC] 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
 [2020-10-01 14:13 UTC] nikic@php.net
-Status: Verified +Status: Closed
 [2020-10-01 15:14 UTC] 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 15:19 UTC] nikic@php.net
-Status: Closed +Status: Re-Opened
 [2020-10-01 15:19 UTC] 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:23 UTC] 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-02 08:37 UTC] 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-02 09:14 UTC] nikic@php.net
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.
 [2020-10-12 11:03 UTC] nikic@php.net
-Status: Re-Opened +Status: Closed
 [2020-10-12 11:03 UTC] nikic@php.net
The PDO SQLite GC fix has landed, so I'm closing this again.
 [2020-10-12 11:08 UTC] php dot net at ss dot st dot tc
Very much appreciated!
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Mon Oct 26 10:01:22 2020 UTC