search

phpv8 / php-v8

PHP extension for V8 JavaScript engine
https://php-v8.readthedocs.io
MIT License
217 stars 14 forks source link

Resolving/rejecting Promise in non-pending state leads to segfault #95

Closed pinepain closed 6 years ago

pinepain commented 6 years ago

After upgrading to v8 6.6.275 tests/PromiseObject_methods.phpt start failing with

==4587== Invalid read of size 8
==4587==    at 0xA3939B7: ??? (in /opt/libv8/lib/libv8.so)
==4587==    by 0xA3C806D: ??? (in /opt/libv8/lib/libv8.so)
==4587==    by 0x9F07A7D: v8::Promise::Resolver::Resolve(v8::Local<v8::Context>, v8::Local<v8::Value>) (in /opt/libv8/lib/libv8.so)
==4587==    by 0x992EF0A: zim_Promise_resolve(_zend_execute_data*, _zval_struct*) (php_v8_promise.cc:71)
==4587==    by 0x453CC5: execute_ex (in /usr/bin/php7.2)
==4587==    by 0x453F8D: zend_execute (in /usr/bin/php7.2)
==4587==    by 0x3A2852: zend_execute_scripts (in /usr/bin/php7.2)
==4587==    by 0x33DAEF: php_execute_script (in /usr/bin/php7.2)
==4587==    by 0x456358: ??? (in /usr/bin/php7.2)
==4587==    by 0x1F2DEB: ??? (in /usr/bin/php7.2)
==4587==    by 0x690A82F: (below main) (libc-start.c:291)

https://travis-ci.org/pinepain/php-v8/builds/344550480

which is triggered when resolve/reject invoked on a promise (or chained promise) which is not in pending state or has more than one promise in chain that isn't in pending state. According to spec and to v8 API documentation this is a bug as calling resolve/reject on non-pending promise should have no impact and from end-user perspective such calls should run like normal one but without any effect.

As I'm really tired of v8 upgrading race and as I don't use Promise API I suggest to those who are affected validate my assumption, ping v8 developers, provide them with a minimal code to reproduce the issue and ping me back when they release a fix. Eventually, I may do it myself, though, I would appreciate any help here or at least feedback.

pinepain commented 6 years ago

Here's the thread to discuss the issue with v8-users https://groups.google.com/forum/#!topic/v8-users/UHgwDEMl4ZI

pinepain commented 6 years ago

The v8-specific issue is fixed in upstream and available since v8 6.6.309.

However, in current php-v8 implementation both Proxy and Resolver are encapsulated into a single object which causes segfaults when attempting to resolve/reject chain of promises one by one in a random order which is never a case when done within v8 runtime, so I reworked PromiseObject into PromiseObject and PromiseObject\ResolverObject. While ResolverObject extends PromiseObject, it allows use to use full API on a single object when promise created from API and at the same time maintain compatibility with what v8 actually does.