Some internal data structures seem to stick around and start filling memory

[RobDonovan]

Environment

• PHP: PHP 7.0.4 (cli) (built: Mar 12 2016 14:07:47) ( ZTS )*
• pthreads: Version => 3.1.7dev
• OS: Ubuntu 14.04 LTS (HVM) (3.13.0-79-generic)

Summary

When using Volatile arrays, it seems that 'unsetting' the array elements doesnt fully remove the data. The thread that unsets them seems fine, var_dump($this) & var_export($this,true) shows that. But the thread that actually set the data still has the data locally somewhere. It doesnt show with var_dump($this), but it does show with var_export($this,true). Not sure why, and I couldn't find what the diff was with dump or export.

If you loop this to make the creator thread create batches of 100000 and then the deleting thread delete them all, and then loop, and leave it running, eventually all memory is consumed.

It seems to be to do with myRec being 'Threaded'. If its not threaded its fine, but I then cant set individual members like in setData1() as it just ignores the set.

Sorry for a not so simple test case, but this was the simplest way I could show it happening in a test case.

(Ignore the unsafe waiting and notifying, its just for the example, and the sleep is just to make it easier to show this happening rather than a more complex wait/notify.)

Reproducing Code

<?php

class myRec extends Threaded {
    public $data1;
    public $data2;
}

class storage extends Threaded {

        private $store = array();

        public function addRecord($key,$data) {
                $this->store[$key] = $data;
        }

        public function delAll() {
                foreach ($this->store as $key => $data) {
                        unset($this->store[$key]);
                }
        }

        public function getData1($key) {
                return $this->store[$key]->data1;
        }
        public function setData1($key,$data) {
                $this->store[$key]->data1 = $data;
        }
}

class myThread extends Thread
{

    private static $type;

    public function __construct($type,$storage)
    {
        self::$type = $type;
        $this->storage = $storage;
    }

    public function run()
    {

        if (self::$type === 1) {
                for ($i=0;$i<5;$i++) {
                        $r = new  myRec();
                        $r->data1 = 'data'.$i;
                        $this->storage->addRecord($i,$r);
                }

                $this->storage->setData1(1,'newData');
                var_dump($this->storage->getData1(1));

                $this->synchronized(function() {$this->wait();});

                echo "CREATING THREADS THIS:\n";
                var_dump($this);

                $thisExport = explode("\n",var_export($this,true));
                foreach ($thisExport as $data) {
                    print 'CREATING THREAD EXPORT|'.$data . "\n";
                }


        }

        if (self::$type === 2) {
                $this->storage->delAll();

                $thisExport = explode("\n",var_export($this,true));
                foreach ($thisExport as $data) {
                    print 'DELETING THREAD EXPORT|'.$data . "\n";
                }


        }
    }
}

$storage = new storage();

$threads[0] = new myThread(1,$storage);
$threads[0]->start();

sleep(3);// Unsafe, but just for this example to let thread 1 add the 5 recs

$threads[1] = new myThread(2,$storage);
$threads[1]->start();
$threads[1]->join();

// Now thread 1 should not have any data
$threads[0]->synchronized(function() use($threads) {$threads[0]->notify();});

?>

Expected Output

string(7) "newData"
DELETING THREAD EXPORT|myThread::__set_state(array(
DELETING THREAD EXPORT|   'storage' =>
DELETING THREAD EXPORT|  storage::__set_state(array(
DELETING THREAD EXPORT|     'store' =>
DELETING THREAD EXPORT|    Volatile::__set_state(array(
DELETING THREAD EXPORT|    )),
DELETING THREAD EXPORT|  )),
DELETING THREAD EXPORT|))
CREATING THREADS THIS:
object(myThread)#1 (1) {
  ["storage"]=>
  object(storage)#5 (1) {
    ["store"]=>
    object(Volatile)#6 (0) {
    }
  }
}
CREATING THREAD EXPORT|myThread::__set_state(array(
CREATING THREAD EXPORT|   'storage' =>
CREATING THREAD EXPORT|  storage::__set_state(array(
CREATING THREAD EXPORT|     'store' =>
CREATING THREAD EXPORT|    Volatile::__set_state(array(
CREATING THREAD EXPORT|    )),
CREATING THREAD EXPORT|  )),
CREATING THREAD EXPORT|))

Actual Output

string(7) "newData"
DELETING THREAD EXPORT|myThread::__set_state(array(
DELETING THREAD EXPORT|   'storage' =>
DELETING THREAD EXPORT|  storage::__set_state(array(
DELETING THREAD EXPORT|     'store' =>
DELETING THREAD EXPORT|    Volatile::__set_state(array(
DELETING THREAD EXPORT|    )),
DELETING THREAD EXPORT|  )),
DELETING THREAD EXPORT|))
CREATING THREADS THIS:
object(myThread)#1 (1) {
  ["storage"]=>
  object(storage)#9 (1) {
    ["store"]=>
    object(Volatile)#10 (0) {
    }
  }
}
CREATING THREAD EXPORT|myThread::__set_state(array(
CREATING THREAD EXPORT|   'storage' =>
CREATING THREAD EXPORT|  storage::__set_state(array(
CREATING THREAD EXPORT|     'store' =>
CREATING THREAD EXPORT|    Volatile::__set_state(array(
CREATING THREAD EXPORT|       0 =>
CREATING THREAD EXPORT|      myRec::__set_state(array(
CREATING THREAD EXPORT|         'data1' => 'data0',
CREATING THREAD EXPORT|         'data2' => NULL,
CREATING THREAD EXPORT|      )),
CREATING THREAD EXPORT|       1 =>
CREATING THREAD EXPORT|      myRec::__set_state(array(
CREATING THREAD EXPORT|         'data1' => 'newData',
CREATING THREAD EXPORT|         'data2' => NULL,
CREATING THREAD EXPORT|      )),
CREATING THREAD EXPORT|       2 =>
CREATING THREAD EXPORT|      myRec::__set_state(array(
CREATING THREAD EXPORT|         'data1' => 'data2',
CREATING THREAD EXPORT|         'data2' => NULL,
CREATING THREAD EXPORT|      )),
CREATING THREAD EXPORT|       3 =>
CREATING THREAD EXPORT|      myRec::__set_state(array(
CREATING THREAD EXPORT|         'data1' => 'data3',
CREATING THREAD EXPORT|         'data2' => NULL,
CREATING THREAD EXPORT|      )),
CREATING THREAD EXPORT|       4 =>
CREATING THREAD EXPORT|      myRec::__set_state(array(
CREATING THREAD EXPORT|         'data1' => 'data4',
CREATING THREAD EXPORT|         'data2' => NULL,
CREATING THREAD EXPORT|      )),
CREATING THREAD EXPORT|    )),
CREATING THREAD EXPORT|  )),
CREATING THREAD EXPORT|))

[RobDonovan]

Maybe a bit more simpler example adapted from your test case volatile-objects.phpt:

<?php

class Member extends Volatile {
        public function method(array $t) {
                $this->thing = $t;
        }

        public function clear() {
                unset($this->thing);
        }
}
class Test extends Thread {
        public function __construct($type,Member $member) {
                $this->type = $type;
                $this->member = $member;
        }
        public function run() {

                if ($this->type === 1) {
                        $members = [];
                        $this->member->method([1,2,3,4,5]);
                        var_dump($this);

                        sleep(5);// Long sleep to allow other thread to delete member/array

                        var_dump($this); // Show member/array has been removed
                        $thisExport = explode("\n",var_export($this,true));  // Show it is still somewhere though
                        foreach ($thisExport as $data) {
                            print 'CREATING THREAD EXPORT|'.$data . "\n";
                        }
                }

                if ($this->type === 2) {
                        $this->member->clear();
                        sleep(5); // Give the other thread time to print $this
                }
        }
}
$member = new Member();

$test = new Test(1,$member);
$test->start();

sleep(2);// Short sleep to allow thread to add member/array

$t2 = new Test(2,$member);
$t2->start();

$test->join();
$t2->join();
?>

[krakjoe]

I have committed a fix to make var_export and var_dump consistent ...

However, the problem isn't going to go away ... when you unset() you can only unset the member in your own thread, you unset it from the shared properties table too, but it is impossible to unset it in another threads standard property store, it will be retained in that store until the object is destroyed.

Let me think about this a while ... it really is impossible to free anything on another contexts heap, but maybe we can do something ...

[krakjoe]

Ok, so I had a think about it ... we can synchronize the store on read of a property, and on synchronized ... so committed these changes ... it means you have to take special action, but volatile things are volatile ... it should not hamper the performance of non-volatile objects ... have a test with that, workable ?

[sirsnyder]

Could it be an option to trigger a cleanup function per thread in userland?

[RobDonovan]

Ok, I see, I can understand that.

I've tried the new version, var_export and var_dump seem the same now.

However, it still seems to consume memory until it maxes out the machine though, if I add 100000 at a time and then remove them all and then repeat.

When you say 'take special action', what do you mean?

I tried wrapping the adds and removes in a synchronized to see if that helped, but it didnt.

[krakjoe]

hmm ... synchronized should have done it ..

solution will probably change, but for now it should cleanup on synchronized or reading a property, which is what I meant by special action ...

I might have messed something up, let me come back with a clear head and look at the problem again tomorrow ...

[RobDonovan]

Ok cool, no probs. When you get the time.

Just to show you what I'm doing, so I dont waste your time if I'm doing something bad.... ;)

I can see memory in 'top' going up and up, and eventually php fatals and says out of memory.

<?php

class myRec extends Threaded {
    public $data1;
    public $data2;
}

class storage extends Threaded {

        private $store = array();

        public function addRecord($key,$data) {
                $this->store[$key] = $data;
        }

        public function delAll() {
                foreach ($this->store as $key => $data) {
                        unset($this->store[$key]);
                }
        }

        public function countRecs() {
                return count($this->store);
        }
}

class myThread extends Thread
{

    private static $type;

    public function __construct($type,$storage)
    {
        self::$type = $type;
        $this->storage = $storage;
    }

    public function run()
    {

        if (self::$type === 1) {
                while (true) {
                @$loop++;
                for ($i=0;$i<30000;$i++) {
                        $r = new  myRec();
                        $r->data1 = 'data'.$i;
                        $this->storage->synchronized(function() use($i,$r,$loop) {
                                $this->storage->addRecord($loop.'|'.$i,$r);
                        });
                }

                sleep(5);

                }
        }

        if (self::$type === 2) {
                while (true) {
                        $this->storage->synchronized(function() {
                                $this->storage->delAll();
                        });

                        sleep(1);
                        print "RECS:".$this->storage->countRecs()."\n";
                }

        }
    }
}

ini_set('memory_limit', '-1');

$storage = new storage();

$threads[0] = new myThread(1,$storage);
$threads[0]->start();

$threads[1] = new myThread(2,$storage);
$threads[1]->start();
$threads[1]->join();

?>

[RobDonovan]

Hi,

Any thoughts or ideas on this one?

I'm having problems with running out of memory quite a bit now.

Thanks.

[krakjoe]

Sorry, I'm not ignoring you, just super busy :(

I'll get to it, when I can ...

[RobDonovan]

Ok sure, thanks. :)

[sirsnyder]

@RobDonovan
Synchronized has to be done on the $store volatile/array to cleanup hanging references. Checkout my fork https://github.com/SirSnyder/pthreads/tree/php/71 with serveral bug fixes.