protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.11k stars 15.43k forks source link

PHP extension crashes when "keep_descriptor_pool_after_request" option is enabled #9215

Closed hakimio closed 1 year ago

hakimio commented 2 years ago

What version of protobuf and what language are you using? Version: v3.19.1 Language: PHP v7.4

What operating system and version? Windows 10

What runtime / compiler are you using NTS Visual C++ 2017 x64 (VC15)

What did you do? Steps to reproduce the behavior:

  1. Enable keep_descriptor_pool_after_request option
  2. Call some route which uses protobuf
  3. Call the route a second time

What did you expect to see No crash.

What did you see instead? The PHP extension is crashing/segfaulting

Anything else we should know about your project / environment PHP Extension Build: NTS, VC15 Http server: NGINX with php-cgi

@haberman @TeBoring

EDIT: attached compiled extension. protobuf_x64_vc15_php7.4_nts.zip

MrMage commented 2 years ago

FYI, I am also encountering crashes when keep_descriptor_pool_after_request is set to 1 in two cases:

MrMage commented 2 years ago

I just found https://github.com/protocolbuffers/protobuf/commit/759a53973690140d0dfae614b6bd190dcbad787f, which might fix this. But it seems like this is not available in any protobuf release yet.

hakimio commented 2 years ago

@MrMage it's part of v3.19.1 release.

MrMage commented 2 years ago

@hakimio thanks for pointing that out! I must have misread the date of the commit. Then this issue seems to be not fixed, yet.

MrMage commented 2 years ago

FYI, this might have been fixed in #9995. I'll keep monitoring to see if I still see this error with protobuf 3.21.1 installed, and if yes, post an update here. But this can likely be closed.

MrMage commented 2 years ago

Update: still seeing the crashes, I'm afraid. Seeing a message in the log:

zend_mm_heap corrupted

And an exception is thrown:

Couldn't find descriptor. Note only generated code may derive from \Google\Protobuf\Internal\Message

The stack trace is in the following method inside generated code, at parent::__construct($data);:

    public function __construct($data = NULL) {
        \GPBMetadata\Sync\Protobuf\AppActivityService::initOnce();
        parent::__construct($data);
    }
haberman commented 2 years ago

Can you create a self-contained repro? These kinds of bugs are really hard to fix at a distance unfortunately.

simonberger commented 3 months ago

Were there any new insights or had this been closed due to inactivity?

I have this error as well after executing unit tests with codeception.

PHP 8.3 and protobuf extension 3.25.3.

haberman commented 3 months ago

It was closed due to inactivity and lack of a repro.

If you can reproduce this issue reliably, please feel free to create a new bug report.

simonberger commented 2 months ago

There is a new issue with a simple reproducer and a PR with a fix. Unfortunately both without any reaction yet. I expect most of you reported the issue here moved forward without the option. If you are still interested in having this fixed you could have a look at the issue #16894 Maybe you are even willing to try out the likely fix https://github.com/protocolbuffers/protobuf/pull/16993 which would be great.