Protobuf: PHP c extension doesn't work with mock well

[bshaffer]

I am receiving a segmentation fault when using the current version of the protobuf C extension for PHP. This is duplicated by this travis test for PHP 5.6 and this travis test for PHP 7.2:

$ vendor/bin/phpunit --coverage-clover=coverage.xml
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
...............................................................  63 / 276 ( 22%)
...../home/travis/.travis/job_stages: line 57: 30853 Segmentation fault      (core dumped) vendor/bin/phpunit --coverage-clover=coverage.xml

I can reproduce this on my local machine for PHP 5.6 and PHP 7.2 also.

This may be fixed by #4102, but strangely enough the 3.5.1 version of the protobuf extension has already been released in PECL. Can someone shed some light on this situation?

In the meantime I'll do some digging to see what the exact line is which causes the fault.

[TeBoring]

Did u add new test to reproduce the bug? We didn't find segment fault in
our Travis test.
On Wed, Dec 27, 2017 at 10:51 Brent Shaffer [e-mail]
wrote:

I am receiving a segmentation fault when using the current version of the
protobuf C extension for PHP. This is duplicated by this travis test for
PHP 5.6 https://travis-ci.org/googleapis/gax-php/jobs/322270222#L2829
and this travis test for PHP 7.2
https://travis-ci.org/googleapis/gax-php/jobs/322270226#L2797:

$ vendor/bin/phpunit --coverage-clover=coverage.xml
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
............................................................... 63 / 276 ( 22%)
...../home/travis/.travis/job_stages: line 57: 30853 Segmentation fault (core dumped) vendor/bin/phpunit --coverage-clover=coverage.xml

I can reproduce this on my local machine for PHP 5.6 and PHP 7.2 also.

This may be fixed by #4102 https://github.com/google/protobuf/pull/4102,
but strangely enough the 3.5.1 version of the protobuf extension has already
been released in PECL https://pecl.php.net/package/protobuf/3.5.1. Can
someone shed some light on this situation?

In the meantime I'll do some digging to see what the exact line is which
causes the fault.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/protobuf/issues/4107, or mute the thread
https://github.com/notifications/unsubscribe-auth/AE9H5e7o7PZiZuulrU3bg4tSh-3Q0IDaks5tEpGtgaJpZM4RNudT
.

[bshaffer]

Here is a test to reproduce the bug:

$ git clone -b json-transport [email protected]:googleapis/gax-php.git && cd gax-php && composer install && vendor/bin/phpunit

This only fails when the protobuf C extension is enabled.

[bshaffer]

Okay I tracked down the issue and was able to reproduce it with the following code:

<?php

class TestSegfault extends Google\Protobuf\Internal\Message
{
    private $foo;

    public function setFoo($foo)
    {
        $this->foo = $foo;
    }
}

$message = new TestSegfault();
$message->setFoo('any value');

Paste that into any file and execute it with PHP when the protobuf extension is enabled and the result will be a segfault.

Is this happening because there is no Metadata being initialized?

[TeBoring]

Please try initialize metadata.
On Wed, Dec 27, 2017 at 11:25 Brent Shaffer [e-mail]
wrote:

Okay I tracked down the issue and was able to reproduce it with the
following code:

foo = $foo; }}$message = new TestSegfault();$message->setFoo('any value');

Paste that into any file and execute it with PHP when the protobuf
extension is enabled and the result will be a segfault.

Is this happening because there is no Metadata being initialized?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/google/protobuf/issues/4107#issuecomment-354165008,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE9H5UtNfGSULIEzo6oDLCVOIA1AeQ1eks5tEpmXgaJpZM4RNudT
.

[bshaffer]

@TeBoring Protobuf shouldn't be segfaulting because metadata hasn't been initialized...

[bshaffer]

It should be mentioned that if the native google/protobuf implementation is used, the segmentation fault does not occur. It's only when using the protobuf C extension that this happens.

[bshaffer]

Here is another way to reproduce it, this time with phpunit:

<?php
// test.php

use Google\Protobuf\Internal\Message;

class ProtobufSegfaultTest extends PHPUnit_Framework_TestCase
{
    public function testProtobufSegfault()
    {
        $mockRequest = $this->getMockBuilder(Message::class)
            ->disableOriginalConstructor()
            ->setMethods(['getPageToken'])
            ->getMock();
        $mockRequest->getPageToken();
    }
}

Now run phpunit on that file:

phpunit test.php

[TeBoring]

As talked offline, c extension doesn't work with mock well. Need to investigate if there is a proper mock way or changes needed for protobuf.

[ebankie]

我也遇到了 php7.1.12 core dumped

[svycka]

@bshaffer I am getting Segmentation fault with protobuf disabled but with only grpc maybe it's grpc problem? When I disable grpc everything works fine even with protobuf enabled.

[mcos]

@TeBoring Has there been any progress on this? I'm trying to upgrade to the PECL library for performance reasons, but my phpunit tests fail when setting a oneof field with the error "Given message does not have correct class".

I'm using both v3.6.0 of the protobuf pecl extension and v3.6.0 of the protobuf composer library. Also using both v1.12.0 of the grpc pecl extension and v1.12.0 of the grpc composer library. Tests work just fine when I don't use the protobuf pecl extension, but message serialization is far slower in the application.

[mcos]

Also, another discovery: well-known types (GPBEmpty, Timestamp, etc) in the PECL extension do not extend Google\Protobuf\Internal\Message (as would be expected).

[afk11]

If people are still getting this - next time it comes up, check you're producing core dumps somewhere you can access them and try the following to see where it's actually crashing:

$ gdb /usr/bin/php
> run my_reproduce_script.php
.. crash at whatever.. 
> bt

[TeBoring]

The underlying problem is that protobuf message cannot be extended.
Q: What does people usually do to mock a final class?

[TeBoring]

We have decided to not support mocking for protobuf message.
Later, we will explicitly make protobuf message final.
If you are currently using mock for protobuf message, please change your code ASAP.