php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #45178 garbage collector and cyclic references
Submitted: 2008-06-04 16:57 UTC Modified: 2008-07-25 07:44 UTC
From: thomas dot jarosch at intra2net dot com Assigned: dmitry (profile)
Status: Closed Package: Reproducible crash
PHP Version: 5.3CVS-2008-06-04 (snap) OS: Linux
Private report: No CVE-ID: None
 [2008-06-04 16:57 UTC] thomas dot jarosch at intra2net dot com
Description:
------------
Hello together,

I'm currently trying to find a heap corruption while using Horde and 
noticed a rather odd behavior. The supplied code is the standard way 
Horde does it singletons. We always used the syntax of "$object = 
&new class" to make it work  with PHP4 and PHP5. 

If I change that to "$object = new class", everything works as 
expected. I've found bug #32845 and noticed what we are doing seems 
wrong, so Horde needs fixing.

The problem gets worse if the class object contains a variable of the 
type "PEAR_Error", which contains cyclic references. Not only does 
the constructor get called every time, the object leaks memory like 
hell, even with PHP 5.3. I've searched through the bugtracker and 
thought the garbage collector now handles cyclic references,
but maybe this is a side-effect of something else going wrong.

Is the memory consumption by design?

Thanks in advance for any comment,
Thomas


Reproduce code:
---------------
<?php

require_once "PEAR.php";

class Horde_History
{
    var $error;

    function &singleton()
    {
        static $history;

        if (!isset($history)) {
            $history = &new Horde_History();
        }

        return $history;
    }

    function Horde_History()
    {
        $this->error = PEAR::raiseError("error");
        echo "Memory usage: " . memory_get_usage() . "\n";
    }
}

for (;;) {
    $a = Horde_History::singleton();
}


Expected result:
----------------
Constant memory usage.

Actual result:
--------------
Increasing memory usage.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2008-06-05 09:13 UTC] thomas dot jarosch at intra2net dot com
Actually, the reproduce code from #33595 also leaks memory.

The statement about Horde is not 100% correct as only some singleton 
functions are affected. I mixed it up with another reference problem 
I fixed a while ago. Nontheless memory is leaked :-)
 [2008-07-15 07:37 UTC] dmitry@php.net
At first, you have a serious issue in your code. "static" variables MUST NOT be asigned by reference, because they are already references. As result the singleton pattern just doesn't work. BTW the bug is really exists. The simplified test case follows(changing "=& new" into "= new" fixes memory corruption).

<?php
class Horde_History {
    function raiseError() {
        return debug_backtrace();
    }

    function Horde_History() {
        $this->error = $this->raiseError();
        echo "Memory usage: " . memory_get_usage() . "\n";
    }
}

for ($i=0;$i<100000;$i++) {
	$a =& new Horde_History();
}
?>
 [2008-07-15 11:42 UTC] dmitry@php.net
The simplest case for this memory corruption.

<?php
class Foo {
    function Foo() {
    	$this->error = array($this,$this);
    }
}
$a =& new Foo();

 [2008-07-16 13:20 UTC] thomas dot jarosch at intra2net dot com
Thanks for looking into this, Dmitry!

So there are actually two problems, one memory corruption reported 
here and one memory leak report in #33595 or are they both memory 
corruptions?
 [2008-07-24 13:38 UTC] dmitry@php.net
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.


 [2008-07-25 07:44 UTC] thomas dot jarosch at intra2net dot com
Thanks, Dmitry! Your fix works fine for both bugs.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Apr 20 02:01:29 2024 UTC