Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings
This repository was archived by the owner on Mar 29, 2024. It is now read-only.

Commit 4d08485

Browse files
Merge pull request #77 from pinepain/fix-external-exception-lost
Store exception object when external exception given, closes #76 
2 parents 530a85f + 46977cf commit 4d08485

File tree

4 files changed

+134
-2
lines changed

4 files changed

+134
-2
lines changed

β€Žsrc/php_v8_exceptions.hβ€Ž

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,10 @@ extern void php_v8_throw_try_catch_exception(php_v8_context_t *php_v8_context, v
5353
php_v8_isolate_limits_maybe_stop_timer((php_v8_context)->php_v8_isolate);\
5454
if ((try_catch).HasCaught()) { \
5555
php_v8_throw_try_catch_exception((php_v8_context), &(try_catch)); \
56+
php_v8_isolate_external_exceptions_maybe_clear((php_v8_context)->php_v8_isolate); \
5657
return; \
57-
}
58+
} \
59+
php_v8_isolate_external_exceptions_maybe_clear((php_v8_context)->php_v8_isolate); \
5860

5961

6062
#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));

β€Žsrc/php_v8_isolate.ccβ€Ž

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,37 @@ static inline void php_v8_isolate_destroy(php_v8_isolate_t *php_v8_isolate) {
6464
}
6565
}
6666

67+
void php_v8_isolate_external_exceptions_maybe_clear(php_v8_isolate_t *php_v8_isolate) {
68+
if (!php_v8_isolate->limits.depth) {
69+
php_v8_isolate->external_exceptions->clear();
70+
}
71+
}
72+
73+
namespace phpv8 {
74+
int ExternalExceptionsStack::getGcCount() {
75+
return static_cast<int>(exceptions.size());
76+
}
77+
void ExternalExceptionsStack::collectGcZvals(zval *& zv) {
78+
for (auto const &item : exceptions) {
79+
ZVAL_COPY_VALUE(zv++, &item);
80+
}
81+
}
82+
void ExternalExceptionsStack::add(zval zv) {
83+
assert(IS_OBJECT == Z_TYPE(zv));
84+
Z_ADDREF(zv);
85+
exceptions.push_back(zv);
86+
assert(exceptions.size() < INT_MAX);
87+
}
88+
void ExternalExceptionsStack::clear() {
89+
for (auto &item : exceptions) {
90+
zval_ptr_dtor(&item);
91+
}
92+
exceptions.clear();
93+
}
94+
ExternalExceptionsStack::~ExternalExceptionsStack() {
95+
clear();
96+
}
97+
}
6798

6899
static HashTable * php_v8_isolate_gc(zval *object, zval **table, int *n) {
69100
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) {
73104
size += php_v8_isolate->weak_function_templates->getGcCount();
74105
size += php_v8_isolate->weak_object_templates->getGcCount();
75106
size += php_v8_isolate->weak_values->getGcCount();
107+
size += php_v8_isolate->external_exceptions->getGcCount();
76108

77109
if (php_v8_isolate->gc_data_count < size) {
78110
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) {
85117
php_v8_isolate->weak_function_templates->collectGcZvals(gc_data);
86118
php_v8_isolate->weak_object_templates->collectGcZvals(gc_data);
87119
php_v8_isolate->weak_values->collectGcZvals(gc_data);
120+
php_v8_isolate->external_exceptions->collectGcZvals(gc_data);
88121

89122
*table = php_v8_isolate->gc_data;
90123
*n = php_v8_isolate->gc_data_count;
@@ -109,6 +142,10 @@ static void php_v8_isolate_free(zend_object *object) {
109142
delete php_v8_isolate->weak_values;
110143
}
111144

145+
if (php_v8_isolate->external_exceptions) {
146+
delete php_v8_isolate->external_exceptions;
147+
}
148+
112149
if (php_v8_isolate->gc_data) {
113150
efree(php_v8_isolate->gc_data);
114151
}
@@ -159,6 +196,7 @@ static zend_object *php_v8_isolate_ctor(zend_class_entry *ce) {
159196
php_v8_isolate->weak_function_templates = new phpv8::PersistentCollection<v8::FunctionTemplate>();
160197
php_v8_isolate->weak_object_templates = new phpv8::PersistentCollection<v8::ObjectTemplate>();
161198
php_v8_isolate->weak_values = new phpv8::PersistentCollection<v8::Value>();
199+
php_v8_isolate->external_exceptions = new phpv8::ExternalExceptionsStack();
162200
new(&php_v8_isolate->key) v8::Persistent<v8::Private>();
163201

164202
php_v8_isolate->std.handlers = &php_v8_isolate_object_handlers;
@@ -409,6 +447,7 @@ static PHP_METHOD(Isolate, getEnteredContext) {
409447
}
410448

411449
static PHP_METHOD(Isolate, throwException) {
450+
zval tmp;
412451
zval *php_v8_context_zv;
413452
zval *php_v8_value_zv;
414453
zval *exception_zv = NULL;
@@ -444,6 +483,9 @@ static PHP_METHOD(Isolate, throwException) {
444483
}
445484

446485
ZVAL_COPY(&php_v8_value->exception, exception_zv);
486+
487+
ZVAL_OBJ(&tmp, &php_v8_value->std);
488+
php_v8_isolate->external_exceptions->add(tmp);
447489
}
448490

449491
isolate->ThrowException(local_value);

β€Žsrc/php_v8_isolate.hβ€Ž

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ typedef struct _php_v8_isolate_t php_v8_isolate_t;
2020
#include "php_v8_exceptions.h"
2121
#include "php_v8_callbacks.h"
2222
#include <v8.h>
23-
#include <map>
23+
#include <vector>
2424

2525
extern "C" {
2626
#include "php.h"
@@ -34,6 +34,7 @@ extern zend_class_entry *php_v8_isolate_class_entry;
3434

3535
inline php_v8_isolate_t * php_v8_isolate_fetch_object(zend_object *obj);
3636
inline v8::Local<v8::Private> php_v8_isolate_get_key_local(php_v8_isolate_t *php_v8_isolate);
37+
extern void php_v8_isolate_external_exceptions_maybe_clear(php_v8_isolate_t *php_v8_isolate);
3738

3839
// TODO: remove or cleanup to use for debug reasons
3940
#define SX(x) #x
@@ -130,6 +131,20 @@ inline v8::Local<v8::Private> php_v8_isolate_get_key_local(php_v8_isolate_t *php
130131
}
131132

132133

134+
namespace phpv8 {
135+
136+
class ExternalExceptionsStack {
137+
public:
138+
int getGcCount();
139+
void collectGcZvals(zval *& zv);
140+
void add(zval zv);
141+
void clear();
142+
~ExternalExceptionsStack();
143+
private:
144+
std::vector<zval> exceptions;
145+
};
146+
}
147+
133148
struct _php_v8_isolate_t {
134149
v8::Isolate *isolate;
135150
v8::Isolate::CreateParams *create_params;
@@ -138,6 +153,7 @@ struct _php_v8_isolate_t {
138153
phpv8::PersistentCollection<v8::FunctionTemplate> *weak_function_templates;
139154
phpv8::PersistentCollection<v8::ObjectTemplate> *weak_object_templates;
140155
phpv8::PersistentCollection<v8::Value> *weak_values;
156+
phpv8::ExternalExceptionsStack *external_exceptions;
141157

142158
v8::Persistent<v8::Private> key;
143159

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
--TEST--
2+
V8\Isolate::throwException() - external exception is not lost when provided with refcount=1
3+
--SKIPIF--
4+
<?php if (!extension_loaded("v8")) print "skip"; ?>
5+
--ENV--
6+
HOME=/tmp/we-need-home-env-var-set-to-load-valgrindrc
7+
--FILE--
8+
<?php
9+
/** @var \Phpv8Testsuite $helper */
10+
$helper = require '.testsuite.php';
11+
12+
require '.v8-helpers.php';
13+
$v8_helper = new PhpV8Helpers($helper);
14+
15+
16+
class TestException extends RuntimeException {
17+
public function __destruct()
18+
{
19+
echo __METHOD__, PHP_EOL;
20+
}
21+
}
22+
23+
$isolate = new \V8\Isolate();
24+
$context = new \V8\Context($isolate);
25+
$v8_helper->injectConsoleLog($context);
26+
27+
$global = $context->globalObject();
28+
29+
$func_tpl = new \V8\FunctionObject($context, function (\V8\FunctionCallbackInfo $info) {
30+
$isolate = $info->getIsolate();
31+
$context = $info->getContext();
32+
$info->getIsolate()->throwException($info->getContext(), \V8\ExceptionManager::createError($context, new \V8\StringValue($isolate, 'test')), new TestException('test'));
33+
});
34+
35+
$global->set($context, new \V8\StringValue($isolate, 'e'), $func_tpl);
36+
37+
try {
38+
$v8_helper->CompileRun($context, 'e()');
39+
} catch (\V8\Exceptions\TryCatchException $e) {
40+
$helper->exception_export($e);
41+
$helper->assert('External exception present', $e->getTryCatch()->getExternalException() instanceof TestException);
42+
$helper->exception_export($e->getTryCatch()->getExternalException());
43+
$e = null;
44+
}
45+
46+
$helper->message('done');
47+
$helper->line();
48+
49+
$helper->header('Run with js try-catch');
50+
$v8_helper->CompileRun($context, 'try {e()} catch(e) {}');
51+
52+
$helper->message('done');
53+
54+
$func_tpl = null;
55+
$global = null;
56+
$context = null;
57+
$v8_helper = null;
58+
59+
$isolate = null;
60+
61+
?>
62+
--EXPECT--
63+
V8\Exceptions\TryCatchException: Error: test
64+
External exception present: ok
65+
TestException: test
66+
TestException::__destruct
67+
done
68+
69+
Run with js try-catch:
70+
----------------------
71+
TestException::__destruct
72+
done

0 commit comments

Comments
(0)

AltStyle γ«γ‚ˆγ£γ¦ε€‰ζ›γ•γ‚ŒγŸγƒšγƒΌγ‚Έ (->γ‚ͺγƒͺγ‚ΈγƒŠγƒ«) /