phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine
http://pecl.php.net/package/v8js
MIT License
1.84k stars 200 forks source link

Seg fault when using an object returned directly via a method #6

Closed beest closed 11 years ago

beest commented 11 years ago

The following code sample is the minimum required to reproduce the problem. As you can see, the problem occurs via __get and via an existing method.

I'm testing with PHP-5.3.10 and libv8.so version 3.17.13.

<?php

class Service
{
    public function method()
    {
        print("method\n");
    }
}

class Container
{
    private $service = null;

    public function __get($name)
    {
        if ($name == 'crash') {
            return new Service;
        } else {
            $this->service = new Service;
            return $this->service;
        }
    }

    public function getService()
    {
        return new Service;
    }
}

$v8Js = new V8Js('PHP');
$v8Js->container = new Container;

$code = <<<EOT

var container = PHP.container;

// This works OK
//container.ok.method();

// These both cause seg fault
container.crash.method();
container.getService().method();

EOT;

$v8Js->executeString($code, 'test');

The seg fault happens during the zend_get_class_entry (via Z_OBJCE_P) call in the php_v8js_php_callback function (line 46 in v8js_convert.cc).

I have limited knowledge of Zend internals, but this looks like an object referencing issue. The pointer that is passed into Z_OBJCE_P is coming via the SetAlignedPointerInInternalField. Perhaps we need to explicitly add a reference to the object and use that in the SetAlignedPointerInInternalField call instead?

I'm willing to help fix this if anybody can provide any insights into why this might be happening.

beest commented 11 years ago

With my limited knowledge, I have been experimenting with this issue and I've managed to prevent the seg fault by forcefully increasing the refcount of the zval.

I've done this with a call to Z_SET_REFCOUNT just before calling SetAlignedPointerInInternalField, and only in the case where the refcount is 1.

...
if (Z_REFCOUNT_P(value) == 1)
{
    Z_SET_REFCOUNT_P(value, 2);
}

newobj->SetAlignedPointerInInternalField(0, (void *) value);
...

I appreciate this might be a horrible hack, but it hopefully sheds some light on a fix for this issue.

beest commented 11 years ago

I've thought about this more, and the above fix might actually be close to the solution that is needed.

The root cause of the bug is when an object is instantiated and returned without being stored in any other variable in PHP land. The refcount of the object will decrease to zero, and the garbage collection will remove the object from memory. This is what causes the seg fault when accessing the object via GetAlignedPointerInInternalField.

In fact, the fix should probably always increase the refcount. Not doing so would potentially cause deeper issues later on when variables in PHP are freed but Javascript references still need to be in memory.

beest commented 11 years ago

So the final fix would be:

Z_SET_REFCOUNT_P(value, Z_REFCOUNT_P(value) + 1);

There may be a macro that does this more elegantly (something like Z_INC_REFCOUNT_P), but I'm not sure what it is.

satoshi75nakamoto commented 11 years ago

Yeah, that makes sense. I'll get a patch for it ready in the next couple days. Thanks, @beest.