diff --git a/src/php_v8_exceptions.h b/src/php_v8_exceptions.h index 739da4a..51ec8f9 100644 --- a/src/php_v8_exceptions.h +++ b/src/php_v8_exceptions.h @@ -53,8 +53,10 @@ extern void php_v8_throw_try_catch_exception(php_v8_context_t *php_v8_context, v php_v8_isolate_limits_maybe_stop_timer((php_v8_context)->php_v8_isolate);\ if ((try_catch).HasCaught()) { \ php_v8_throw_try_catch_exception((php_v8_context), &(try_catch)); \ + php_v8_isolate_external_exceptions_maybe_clear((php_v8_context)->php_v8_isolate); \ return; \ - } + } \ + php_v8_isolate_external_exceptions_maybe_clear((php_v8_context)->php_v8_isolate); \ #define PHP_V8_TRY_CATCH_EXCEPTION_STORE_ISOLATE(to_zval, from_isolate_zv) zend_update_property(php_v8_try_catch_exception_class_entry, (to_zval), ZEND_STRL("isolate"), (from_isolate_zv)); diff --git a/src/php_v8_isolate.cc b/src/php_v8_isolate.cc index 845b298..bfd1a8a 100644 --- a/src/php_v8_isolate.cc +++ b/src/php_v8_isolate.cc @@ -64,6 +64,37 @@ static inline void php_v8_isolate_destroy(php_v8_isolate_t *php_v8_isolate) { } } +void php_v8_isolate_external_exceptions_maybe_clear(php_v8_isolate_t *php_v8_isolate) { + if (!php_v8_isolate->limits.depth) { + php_v8_isolate->external_exceptions->clear(); + } +} + +namespace phpv8 { + int ExternalExceptionsStack::getGcCount() { + return static_cast(exceptions.size()); + } + void ExternalExceptionsStack::collectGcZvals(zval *& zv) { + for (auto const &item : exceptions) { + ZVAL_COPY_VALUE(zv++, &item); + } + } + void ExternalExceptionsStack::add(zval zv) { + assert(IS_OBJECT == Z_TYPE(zv)); + Z_ADDREF(zv); + exceptions.push_back(zv); + assert(exceptions.size() < INT_MAX); + } + void ExternalExceptionsStack::clear() { + for (auto &item : exceptions) { + zval_ptr_dtor(&item); + } + exceptions.clear(); + } + ExternalExceptionsStack::~ExternalExceptionsStack() { + clear(); + } +} static HashTable * php_v8_isolate_gc(zval *object, zval **table, int *n) { PHP_V8_ISOLATE_FETCH_INTO(object, php_v8_isolate); @@ -73,6 +104,7 @@ static HashTable * php_v8_isolate_gc(zval *object, zval **table, int *n) { size += php_v8_isolate->weak_function_templates->getGcCount(); size += php_v8_isolate->weak_object_templates->getGcCount(); size += php_v8_isolate->weak_values->getGcCount(); + size += php_v8_isolate->external_exceptions->getGcCount(); if (php_v8_isolate->gc_data_count < size) { php_v8_isolate->gc_data = (zval *)safe_erealloc(php_v8_isolate->gc_data, size, sizeof(zval), 0); @@ -85,6 +117,7 @@ static HashTable * php_v8_isolate_gc(zval *object, zval **table, int *n) { php_v8_isolate->weak_function_templates->collectGcZvals(gc_data); php_v8_isolate->weak_object_templates->collectGcZvals(gc_data); php_v8_isolate->weak_values->collectGcZvals(gc_data); + php_v8_isolate->external_exceptions->collectGcZvals(gc_data); *table = php_v8_isolate->gc_data; *n = php_v8_isolate->gc_data_count; @@ -109,6 +142,10 @@ static void php_v8_isolate_free(zend_object *object) { delete php_v8_isolate->weak_values; } + if (php_v8_isolate->external_exceptions) { + delete php_v8_isolate->external_exceptions; + } + if (php_v8_isolate->gc_data) { efree(php_v8_isolate->gc_data); } @@ -159,6 +196,7 @@ static zend_object *php_v8_isolate_ctor(zend_class_entry *ce) { php_v8_isolate->weak_function_templates = new phpv8::PersistentCollection(); php_v8_isolate->weak_object_templates = new phpv8::PersistentCollection(); php_v8_isolate->weak_values = new phpv8::PersistentCollection(); + php_v8_isolate->external_exceptions = new phpv8::ExternalExceptionsStack(); new(&php_v8_isolate->key) v8::Persistent(); php_v8_isolate->std.handlers = &php_v8_isolate_object_handlers; @@ -409,6 +447,7 @@ static PHP_METHOD(Isolate, getEnteredContext) { } static PHP_METHOD(Isolate, throwException) { + zval tmp; zval *php_v8_context_zv; zval *php_v8_value_zv; zval *exception_zv = NULL; @@ -444,6 +483,9 @@ static PHP_METHOD(Isolate, throwException) { } ZVAL_COPY(&php_v8_value->exception, exception_zv); + + ZVAL_OBJ(&tmp, &php_v8_value->std); + php_v8_isolate->external_exceptions->add(tmp); } isolate->ThrowException(local_value); diff --git a/src/php_v8_isolate.h b/src/php_v8_isolate.h index 6582783..fa2fb2a 100644 --- a/src/php_v8_isolate.h +++ b/src/php_v8_isolate.h @@ -20,7 +20,7 @@ typedef struct _php_v8_isolate_t php_v8_isolate_t; #include "php_v8_exceptions.h" #include "php_v8_callbacks.h" #include -#include +#include extern "C" { #include "php.h" @@ -34,6 +34,7 @@ extern zend_class_entry *php_v8_isolate_class_entry; inline php_v8_isolate_t * php_v8_isolate_fetch_object(zend_object *obj); inline v8::Local php_v8_isolate_get_key_local(php_v8_isolate_t *php_v8_isolate); +extern void php_v8_isolate_external_exceptions_maybe_clear(php_v8_isolate_t *php_v8_isolate); // TODO: remove or cleanup to use for debug reasons #define SX(x) #x @@ -130,6 +131,20 @@ inline v8::Local php_v8_isolate_get_key_local(php_v8_isolate_t *php } +namespace phpv8 { + + class ExternalExceptionsStack { + public: + int getGcCount(); + void collectGcZvals(zval *& zv); + void add(zval zv); + void clear(); + ~ExternalExceptionsStack(); + private: + std::vector exceptions; + }; +} + struct _php_v8_isolate_t { v8::Isolate *isolate; v8::Isolate::CreateParams *create_params; @@ -138,6 +153,7 @@ struct _php_v8_isolate_t { phpv8::PersistentCollection *weak_function_templates; phpv8::PersistentCollection *weak_object_templates; phpv8::PersistentCollection *weak_values; + phpv8::ExternalExceptionsStack *external_exceptions; v8::Persistent key; diff --git a/tests/Isolate_throwException_with_external_preserved.phpt b/tests/Isolate_throwException_with_external_preserved.phpt new file mode 100644 index 0000000..5ac6a68 --- /dev/null +++ b/tests/Isolate_throwException_with_external_preserved.phpt @@ -0,0 +1,72 @@ +--TEST-- +V8\Isolate::throwException() - external exception is not lost when provided with refcount=1 +--SKIPIF-- + +--ENV-- +HOME=/tmp/we-need-home-env-var-set-to-load-valgrindrc +--FILE-- +injectConsoleLog($context); + +$global = $context->globalObject(); + +$func_tpl = new \V8\FunctionObject($context, function (\V8\FunctionCallbackInfo $info) { + $isolate = $info->getIsolate(); + $context = $info->getContext(); + $info->getIsolate()->throwException($info->getContext(), \V8\ExceptionManager::createError($context, new \V8\StringValue($isolate, 'test')), new TestException('test')); +}); + +$global->set($context, new \V8\StringValue($isolate, 'e'), $func_tpl); + +try { + $v8_helper->CompileRun($context, 'e()'); +} catch (\V8\Exceptions\TryCatchException $e) { + $helper->exception_export($e); + $helper->assert('External exception present', $e->getTryCatch()->getExternalException() instanceof TestException); + $helper->exception_export($e->getTryCatch()->getExternalException()); + $e = null; +} + +$helper->message('done'); +$helper->line(); + +$helper->header('Run with js try-catch'); +$v8_helper->CompileRun($context, 'try {e()} catch(e) {}'); + +$helper->message('done'); + +$func_tpl = null; +$global = null; +$context = null; +$v8_helper = null; + +$isolate = null; + +?> +--EXPECT-- +V8\Exceptions\TryCatchException: Error: test +External exception present: ok +TestException: test +TestException::__destruct +done + +Run with js try-catch: +---------------------- +TestException::__destruct +done