Skip to content

Commit 8a39efa

Browse files
stesiestesie
authored
Merge pull request #545 from redbullmarky/php8
Fixing memory leaks and deprecations
2 parents dde1c5a + 7df3e43 commit 8a39efa

7 files changed

Lines changed: 75 additions & 22 deletions

File tree

‎.github/workflows/build-test.yml‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737

3838
steps:
3939
- name: Checkout code
40-
uses: actions/checkout@v2
40+
uses: actions/checkout@v4
4141

4242
- name: Setup PHP
4343
uses: shivammathur/setup-php@v2
@@ -112,15 +112,21 @@ jobs:
112112

113113
steps:
114114
- name: Checkout code
115-
uses: actions/checkout@v2
115+
uses: actions/checkout@v4
116116

117117
- name: Setup latest Alpine Linux
118118
uses: jirutka/setup-alpine@v1
119119

120120
- name: Install dependencies
121121
run: |
122122
cat /etc/alpine-release
123-
apk add php83-dev nodejs-dev g++ make
123+
apk update
124+
PHP_DEV=$(apk search -q 'php[0-9]*-dev' | grep -E '^php[0-9]+-dev$' | sort -V | tail -1)
125+
echo "Installing $PHP_DEV"
126+
apk add $PHP_DEV nodejs-dev g++ make
127+
PHP_VER=$(echo $PHP_DEV | grep -oE '[0-9]+')
128+
[ ! -e /usr/bin/phpize ] && ln -s /usr/bin/phpize$PHP_VER /usr/bin/phpize
129+
[ ! -e /usr/bin/php-config ] && ln -s /usr/bin/php-config$PHP_VER /usr/bin/php-config
124130
shell: alpine.sh --root {0}
125131

126132
- name: Build extension

‎v8js_array_access.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ static zval v8js_array_access_dispatch(zend_object *object, const char *method_n
5959
V8JS_INTERCEPTED v8js_array_access_getter(uint32_t index, const v8::PropertyCallbackInfo<v8::Value>& info) /* {{{ */
6060
{
6161
v8::Isolate *isolate = info.GetIsolate();
62-
v8::Local<v8::Object> self = info.Holder();
62+
v8::Local<v8::Object> self = info.This();
6363

6464
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
6565

@@ -83,7 +83,7 @@ V8JS_INTERCEPTED v8js_array_access_setter(uint32_t index, v8::Local<v8::Value> v
8383
const V8JS_SETTER_PROPERTY_CALLBACK_INFO &info) /* {{{ */
8484
{
8585
v8::Isolate *isolate = info.GetIsolate();
86-
v8::Local<v8::Object> self = info.Holder();
86+
v8::Local<v8::Object> self = info.This();
8787

8888
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
8989

@@ -157,7 +157,7 @@ static bool v8js_array_access_isset_p(zend_object *object, int index) /* {{{ */
157157
static void v8js_array_access_length(v8::Local<v8::String> property, const v8::PropertyCallbackInfo<v8::Value>& info) /* {{{ */
158158
{
159159
v8::Isolate *isolate = info.GetIsolate();
160-
v8::Local<v8::Object> self = info.Holder();
160+
v8::Local<v8::Object> self = info.This();
161161

162162
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
163163

@@ -169,7 +169,7 @@ static void v8js_array_access_length(v8::Local<v8::String> property, const v8::P
169169
V8JS_INTERCEPTED v8js_array_access_deleter(uint32_t index, const v8::PropertyCallbackInfo<v8::Boolean>& info) /* {{{ */
170170
{
171171
v8::Isolate *isolate = info.GetIsolate();
172-
v8::Local<v8::Object> self = info.Holder();
172+
v8::Local<v8::Object> self = info.This();
173173

174174
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
175175

@@ -187,7 +187,7 @@ V8JS_INTERCEPTED v8js_array_access_deleter(uint32_t index, const v8::PropertyCal
187187
V8JS_INTERCEPTED v8js_array_access_query(uint32_t index, const v8::PropertyCallbackInfo<v8::Integer>& info) /* {{{ */
188188
{
189189
v8::Isolate *isolate = info.GetIsolate();
190-
v8::Local<v8::Object> self = info.Holder();
190+
v8::Local<v8::Object> self = info.This();
191191

192192
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
193193

@@ -206,7 +206,7 @@ V8JS_INTERCEPTED v8js_array_access_query(uint32_t index, const v8::PropertyCallb
206206
void v8js_array_access_enumerator(const v8::PropertyCallbackInfo<v8::Array>& info) /* {{{ */
207207
{
208208
v8::Isolate *isolate = info.GetIsolate();
209-
v8::Local<v8::Object> self = info.Holder();
209+
v8::Local<v8::Object> self = info.This();
210210

211211
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
212212

@@ -240,7 +240,7 @@ V8JS_INTERCEPTED v8js_array_access_named_getter(v8::Local<v8::Name> property_nam
240240
return V8JS_INTERCEPTED_YES;
241241
}
242242

243-
v8::Local<v8::Value> ret_value = v8js_named_property_callback(info.GetIsolate(), info.Holder(), property, V8JS_PROP_GETTER);
243+
v8::Local<v8::Value> ret_value = v8js_named_property_callback(info.GetIsolate(), info.This(), property, V8JS_PROP_GETTER);
244244

245245
if(ret_value.IsEmpty()) {
246246
v8::Local<v8::Array> arr = v8::Array::New(isolate);

‎v8js_class.cc‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,44 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {
7373
#endif /** USE_INTERNAL_ALLOCATOR */
7474

7575

76+
static HashTable *v8js_get_gc(zend_object *object, zval **table, int *n) /* {{{ */
77+
{
78+
v8js_ctx *c = v8js_ctx_fetch_object(object);
79+
80+
int count = 3 + (int)c->weak_objects.size();
81+
82+
if (c->gc_buffer_size < count) {
83+
if (c->gc_buffer) efree(c->gc_buffer);
84+
c->gc_buffer = (zval *)safe_emalloc(count, sizeof(zval), 0);
85+
c->gc_buffer_size = count;
86+
}
87+
88+
int i = 0;
89+
ZVAL_COPY_VALUE(&c->gc_buffer[i++], &c->module_normaliser);
90+
ZVAL_COPY_VALUE(&c->gc_buffer[i++], &c->module_loader);
91+
ZVAL_COPY_VALUE(&c->gc_buffer[i++], &c->exception_filter);
92+
93+
for (auto &pair : c->weak_objects) {
94+
ZVAL_OBJ(&c->gc_buffer[i++], pair.first);
95+
}
96+
97+
*table = c->gc_buffer;
98+
*n = i;
99+
return NULL;
100+
}
101+
/* }}} */
102+
103+
76104
static void v8js_free_storage(zend_object *object) /* {{{ */
77105
{
78106
v8js_ctx *c = v8js_ctx_fetch_object(object);
79107

80108
zend_object_std_dtor(&c->std);
81109

110+
if (c->gc_buffer) {
111+
efree(c->gc_buffer);
112+
}
113+
82114
zval_ptr_dtor(&c->module_normaliser);
83115
zval_ptr_dtor(&c->module_loader);
84116
zval_ptr_dtor(&c->exception_filter);
@@ -316,6 +348,9 @@ static PHP_METHOD(V8Js, __construct)
316348
ZVAL_NULL(&c->module_loader);
317349
ZVAL_NULL(&c->exception_filter);
318350

351+
c->gc_buffer = NULL;
352+
c->gc_buffer_size = 0;
353+
319354
// Isolate execution
320355
v8::Isolate *isolate = c->isolate;
321356
v8::Locker locker(isolate);
@@ -688,7 +723,9 @@ static PHP_METHOD(V8Js, setModuleNormaliser)
688723
}
689724

690725
c = Z_V8JS_CTX_OBJ_P(getThis());
726+
zval tmp = c->module_normaliser;
691727
ZVAL_COPY(&c->module_normaliser, callable);
728+
zval_ptr_dtor(&tmp);
692729
}
693730
/* }}} */
694731

@@ -704,7 +741,9 @@ static PHP_METHOD(V8Js, setModuleLoader)
704741
}
705742

706743
c = Z_V8JS_CTX_OBJ_P(getThis());
744+
zval tmp = c->module_loader;
707745
ZVAL_COPY(&c->module_loader, callable);
746+
zval_ptr_dtor(&tmp);
708747
}
709748
/* }}} */
710749

@@ -719,7 +758,9 @@ static PHP_METHOD(V8Js, setExceptionFilter)
719758
}
720759

721760
v8js_ctx *c = Z_V8JS_CTX_OBJ_P(getThis());
761+
zval tmp = c->exception_filter;
722762
ZVAL_COPY(&c->exception_filter, callable);
763+
zval_ptr_dtor(&tmp);
723764
}
724765
/* }}} */
725766

@@ -815,7 +856,7 @@ static PHP_METHOD(V8Js, setAverageObjectSize)
815856
static void v8js_persistent_zval_ctor(zval *p) /* {{{ */
816857
{
817858
assert(Z_TYPE_P(p) == IS_STRING);
818-
Z_STR_P(p) = zend_string_dup(Z_STR_P(p), 1);
859+
Z_STR_P(p) = zend_string_init(ZSTR_VAL(Z_STR_P(p)), ZSTR_LEN(Z_STR_P(p)), 1);
819860
}
820861
/* }}} */
821862

@@ -1077,6 +1118,7 @@ PHP_MINIT_FUNCTION(v8js_class) /* {{{ */
10771118
v8js_object_handlers.clone_obj = NULL;
10781119
v8js_object_handlers.write_property = v8js_write_property;
10791120
v8js_object_handlers.unset_property = v8js_unset_property;
1121+
v8js_object_handlers.get_gc = v8js_get_gc;
10801122

10811123
/* V8Js Class Constants */
10821124
zend_declare_class_constant_string(php_ce_v8js, ZEND_STRL("V8_VERSION"), PHP_V8_VERSION);

‎v8js_class.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ struct v8js_ctx {
7474
zval zval_snapshot_blob;
7575
v8::StartupData snapshot_blob;
7676

77+
zval *gc_buffer;
78+
int gc_buffer_size;
79+
7780
zend_object std;
7881
};
7982
/* }}} */

‎v8js_exceptions.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ void v8js_create_script_exception(zval *return_value, v8::Isolate *isolate, v8::
9797
if(try_catch->Exception()->IsObject() && try_catch->Exception()->ToObject(context).ToLocal(&error_object) && error_object->InternalFieldCount() == 2) {
9898
zend_object *php_exception = reinterpret_cast<zend_object *>(error_object->GetAlignedPointerFromInternalField(1));
9999

100-
zend_class_entry *exception_ce = zend_exception_get_default();
100+
zend_class_entry *exception_ce = zend_ce_exception;
101101
if (instanceof_function(php_exception->ce, exception_ce)) {
102102
#ifdef GC_ADDREF
103103
GC_ADDREF(php_exception);

‎v8js_object_export.cc‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ v8::Local<v8::Value> v8js_propagate_exception(v8js_ctx *ctx) /* {{{ */
5050
}
5151

5252
zval tmp_zv;
53+
ZVAL_UNDEF(&tmp_zv);
5354

5455
if (Z_TYPE(ctx->exception_filter) != IS_NULL) {
5556
zval params[1];
@@ -65,6 +66,7 @@ v8::Local<v8::Value> v8js_propagate_exception(v8js_ctx *ctx) /* {{{ */
6566
} else {
6667
return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate));
6768
}
69+
zval_ptr_dtor(&tmp_zv);
6870
} else {
6971
ZVAL_OBJ(&tmp_zv, EG(exception));
7072
return_value = ctx->isolate->ThrowException(zval_to_v8js(&tmp_zv, ctx->isolate));
@@ -212,7 +214,7 @@ static void v8js_call_php_func(zend_object *object, zend_function *method_ptr, c
212214
return_value = v8js_propagate_exception(ctx);
213215
} else if (Z_TYPE(retval) == IS_OBJECT && Z_OBJ(retval) == object) {
214216
// special case: "return $this"
215-
return_value = info.Holder();
217+
return_value = info.This();
216218
} else {
217219
return_value = zval_to_v8js(&retval, isolate);
218220
}
@@ -227,7 +229,7 @@ static void v8js_call_php_func(zend_object *object, zend_function *method_ptr, c
227229
/* Callback for PHP methods and functions */
228230
void v8js_php_callback(const v8::FunctionCallbackInfo<v8::Value>& info) /* {{{ */
229231
{
230-
v8::Local<v8::Object> self = info.Holder();
232+
v8::Local<v8::Object> self = info.This();
231233

232234
zend_object *object = reinterpret_cast<zend_object *>(self->GetAlignedPointerFromInternalField(1));
233235
zend_function *method_ptr;
@@ -364,7 +366,7 @@ static void v8js_named_property_enumerator(const v8::PropertyCallbackInfo<v8::Ar
364366
v8::Isolate *isolate = info.GetIsolate();
365367
v8::Local<v8::Context> v8_context = isolate->GetEnteredOrMicrotaskContext();
366368

367-
v8::Local<v8::Object> self = info.Holder();
369+
v8::Local<v8::Object> self = info.This();
368370
v8::Local<v8::Array> result = v8::Array::New(isolate, 0);
369371
uint32_t result_len = 0;
370372

@@ -466,7 +468,7 @@ static void v8js_invoke_callback(const v8::FunctionCallbackInfo<v8::Value>& info
466468
v8::Isolate *isolate = info.GetIsolate();
467469
v8::Local<v8::Context> v8_context = isolate->GetEnteredOrMicrotaskContext();
468470

469-
v8::Local<v8::Object> self = info.Holder();
471+
v8::Local<v8::Object> self = info.This();
470472
v8::Local<v8::Function> cb = v8::Local<v8::Function>::Cast(info.Data());
471473
int argc = info.Length(), i;
472474
v8::Local<v8::Value> *argv = static_cast<v8::Local<v8::Value> *>(alloca(sizeof(v8::Local<v8::Value>) * argc));
@@ -511,7 +513,7 @@ static void v8js_fake_call_impl(const v8::FunctionCallbackInfo<v8::Value>& info)
511513
v8::Isolate *isolate = info.GetIsolate();
512514
v8::Local<v8::Context> v8_context = isolate->GetEnteredOrMicrotaskContext();
513515

514-
v8::Local<v8::Object> self = info.Holder();
516+
v8::Local<v8::Object> self = info.This();
515517
v8::Local<v8::Value> return_value = V8JS_NULL;
516518

517519
char *error;
@@ -859,7 +861,7 @@ v8::Local<v8::Value> v8js_named_property_callback(v8::Isolate *isolate, v8::Loca
859861

860862
static V8JS_INTERCEPTED v8js_named_property_getter(v8::Local<v8::Name> property, const v8::PropertyCallbackInfo<v8::Value> &info) /* {{{ */
861863
{
862-
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.Holder(), property, V8JS_PROP_GETTER);
864+
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.This(), property, V8JS_PROP_GETTER);
863865

864866
if (r.IsEmpty()) {
865867
return V8JS_INTERCEPTED_NO;
@@ -872,7 +874,7 @@ static V8JS_INTERCEPTED v8js_named_property_getter(v8::Local<v8::Name> property,
872874

873875
static V8JS_INTERCEPTED v8js_named_property_setter(v8::Local<v8::Name> property, v8::Local<v8::Value> value, const V8JS_SETTER_PROPERTY_CALLBACK_INFO &info) /* {{{ */
874876
{
875-
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.Holder(), property, V8JS_PROP_SETTER, value);
877+
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.This(), property, V8JS_PROP_SETTER, value);
876878
#if PHP_V8_HAS_INTERCEPTED
877879
return r.IsEmpty() ? v8::Intercepted::kNo : v8::Intercepted::kYes;
878880
#else
@@ -883,7 +885,7 @@ static V8JS_INTERCEPTED v8js_named_property_setter(v8::Local<v8::Name> property,
883885

884886
static V8JS_INTERCEPTED v8js_named_property_query(v8::Local<v8::Name> property, const v8::PropertyCallbackInfo<v8::Integer> &info) /* {{{ */
885887
{
886-
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.Holder(), property, V8JS_PROP_QUERY);
888+
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.This(), property, V8JS_PROP_QUERY);
887889
if (r.IsEmpty()) {
888890
return V8JS_INTERCEPTED_NO;
889891
}
@@ -901,7 +903,7 @@ static V8JS_INTERCEPTED v8js_named_property_query(v8::Local<v8::Name> property,
901903

902904
static V8JS_INTERCEPTED v8js_named_property_deleter(v8::Local<v8::Name> property, const v8::PropertyCallbackInfo<v8::Boolean> &info) /* {{{ */
903905
{
904-
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.Holder(), property, V8JS_PROP_DELETER);
906+
v8::Local<v8::Value> r = v8js_named_property_callback(info.GetIsolate(), info.This(), property, V8JS_PROP_DELETER);
905907
if (r.IsEmpty()) {
906908
return V8JS_INTERCEPTED_NO;
907909
}

‎v8js_variables.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ void v8js_register_accessors(std::vector<v8js_accessor_ctx*> *accessor_list, v8:
7676

7777
// Create context to store accessor data
7878
v8js_accessor_ctx *ctx = (v8js_accessor_ctx *)emalloc(sizeof(v8js_accessor_ctx));
79-
ctx->variable_name = zend_string_copy(Z_STR_P(item));
79+
ctx->variable_name = zend_string_init(ZSTR_VAL(Z_STR_P(item)), ZSTR_LEN(Z_STR_P(item)), 1);
8080
ctx->isolate = isolate;
8181

8282
/* Set the variable fetch callback for given symbol on named property */

0 commit comments

Comments
 (0)