v8js leaks memory if objects are returned to javascript

[stesie]

Hi there,

if I call a PHP function from JavaScript context and the PHP function returns a object, the object seems not to be correctly garbage collected even if the passed back object is not stored at all.

Code snippet to reproduce:

<?php

ini_set("v8js.flags", "--trace_gc");
$v8 = new V8Js();

class Failer {
    function call() {
        return new stdClass();
    }   
}

$v8->failer = new Failer();

$v8->executeString('
        for(;;) {
            PHP.failer.call();
        }
    ');

... after running for some twenty seconds the memory consumption hits roughly 1,5 GB, then the loop performs very poorly and v8 garbage collects on almost every instruction ... however it doesn't get rid of all those objects ...

No problem if you pass back an array or immediate values from the function though ...

I've just started using v8js the other day, neither am I a PHP "backend" developer, hence I currently have no clue on how to fix that...

cheers stesie

[beest]

Thanks @stesie.

This is an interesting one, because I fixed the opposite problem recently. Objects returned to Javascript were being garbage collected by PHP, which in turn caused a seg fault to occur when the objects were accessed or methods called from JS.

Does your Pull Request fix this one, or is it still an issue?

[stesie]

No, the pull request does address another issue.

My theory about this particular issue currently is (once more I'm still not very much into the topic, just gdb'ed around some time), that the problem lies in php_v8js_hash_to_jsobj, where a v8::FunctionTemplate is created for each and every object passed back, which is used to create the actual JavaScript object in turn. However the FunctionTemplate persists and subsequent calls lead to further function templates.

I wonder (but have not yet had the time to try out and test further) if a function template should be created only once per class (e.g. based on its name) and reused for subsequent calls to create object instances.

[stesie]

Just to let you know, that I'm working on the topic: https://github.com/stesie/v8js/commit/678bd1fe866ba5fc3c4c9f05aafb608794ad0d06

... still needs some more work, but I'll follow up :-)

So far it's without all the v8::Isolate fluff, since it doesn't work reliably wrt. memory leak monitoring

For the moment the snippet from above doesn't seem to leak memory on JS side, at least the GC Trace log does tell so. But still it leaks memory if you monitor the memory usage of the PHP process, hence probably on PHP side. I assume that there's still a flaw with the Z_ADDREF_P call introduced with issue #6, which adds a reference on the PHP object but never releases it. I assume we need to wrap the freshly created object in a v8::Persistent, make it weak and attach a callback that releases the reference on the PHP object, once the associated JS object is garbage collected.

[beest]

This looks really good.

Question: is it necessary to keep the FunctionTemplate after NewInstance has been called? If not then the quick fix here could be to free the FunctionTemplate after it is used to create a new instance.

Good point re. the Z_ADDREF_P fix. The solution you've proposed sounds spot on, and should fix the memory leak caused there.

[stesie]

@beest I think you cannot dispose a FunctionTemplate. The handle is passed back as a v8::Local hence there is no Dispose method. After all you can't get rid of created functions as well, they always live until the end of the context.

[stesie]

I'm closing this issue myself in favour of the pull request I just filed.