nikic / scalar_objects

Extension that adds support for method calls on primitive types in PHP
MIT License
1.13k stars 44 forks source link

Segmentation fault with PHPUnit #13

Closed deeky666 closed 9 years ago

deeky666 commented 10 years ago

I noticed that scalar_objects doesn't play very nicely with PHPUnit. Each time a NOTICE, WARNING, ERROR etc. is raised inside a scalar type handler from a native PHP function, PHPUnit tests segfault. I provided an example repo to reproduce this. Just run the test suite and you'll see. Does it fall under the "limitations" described in the README or is it a bug that can be fixed? I tested this with PHP 5.5 and traced the segmentation fault down to this line: Program received signal SIGSEGV, Segmentation fault.

nikic commented 10 years ago

The issue here is that PHPUnit uses a [$obj, $method] callback for the error handler, which doesn't work for a non-object $this. So yeah, this is the third "limitations" point and I don't think it can be fixed while keeping the current approach :/

Probably this is a good time to try and get #12 to work for PHP <5.6. That approach would resolve all $this related segfaults.

deeky666 commented 10 years ago

Hmmmm :( Maybe you are right and it would be a good idea to try another approach that does not have those limitations. Another issue I encountered is that you seemingly cannot use mutable APIs on primitives which are class properties. First I thought it is the issue you describe as second limitation in the README but it states that it is not possible for PHP >= 5.6, but I am using PHP 5.5. Does it maybe have to do with by reference values?

class Foo
{
    private $array = array();

    public function bar($value)
    {
        $this->array->push($value); // does not update $this->array

        $array = $this->array;
        $array->push($value);
        $this->array = $array; // updates $this->array
    }
}

Is that case also a known limitation for PHP < 5.6 or is it a bug?

nikic commented 10 years ago

I wasn't aware that this didn't work before in PHP < 5.6, but it's not much of a surprise either. The problem is that by-reference modifications require that the value is fetched in a certain mode (here FETCH_OBJ_W). However method calls fetch the object using FETCH_OBJ_R (because it is not modified in the by-reference sense of the word). So effectively you're just modifying a copy here.

That it worked for array accesses ($array[0]->push()) in PHP < 5.6 was only incidental, because the fetch code was using AI_SET_PTR. For object properties ($this->array->push()) that is not the case, because here the by-ref modification requires going through totally different code (get_property_ptr_ptr instead of read_property).

So I guess the conclusion is: Having methods that (robustly) modify the array just isn't technically possible. I think it wouldn't even be possible if implemented in PHP core (after all, we can't just always fetch the object of a method call in OBJ_W mode). So this entire approach is limited to an immutable API :(

deeky666 commented 10 years ago

Hmmm okay :( But thanks for clarifying!

nikic commented 9 years ago

The new approach using an extra param is implemented now, so array callbacks should now work properly.