Cleanup callback info creation and invalidation

Author: pinepain

src/php_v8_callback_info.cc

@@ -31,20 +31,17 @@ php_v8_callback_info_t * php_v8_callback_info_fetch_object(zend_object *obj) {
     return (php_v8_callback_info_t *)((char *)obj - XtOffsetOf(php_v8_callback_info_t, std));
 }
 
-void php_v8_callback_info_invalidate(zval *val) {
-    PHP_V8_CALLBACK_INFO_FETCH_INTO(val, php_v8_callback_info);
-    php_v8_callback_info->php_v8_isolate = NULL;
-
-    if (!Z_ISUNDEF(php_v8_callback_info->retval)) {
-        php_v8_return_value_mark_expired(&php_v8_callback_info->retval);
+void php_v8_callback_info_invalidate(php_v8_callback_info_t *php_v8_callback_info) {
+    if (php_v8_callback_info->php_v8_return_value) {
+        php_v8_return_value_mark_expired(php_v8_callback_info->php_v8_return_value);
     }
 }
 
 
 static HashTable * php_v8_callback_info_gc(zval *object, zval **table, int *n) {
     PHP_V8_CALLBACK_INFO_FETCH_INTO(object, php_v8_callback_info);
 
-    int size = 2; // args + retval
+    int size = 2; // args + php_v8_return_value->this_ptr
 
     if (php_v8_callback_info->gc_data_count < size) {
         php_v8_callback_info->gc_data = (zval *)safe_erealloc(php_v8_callback_info->gc_data, size, sizeof(zval), 0);
@@ -53,7 +50,7 @@ static HashTable * php_v8_callback_info_gc(zval *object, zval **table, int *n) {
     php_v8_callback_info->gc_data_count = size;
 
     ZVAL_COPY_VALUE(&php_v8_callback_info->gc_data[0], &php_v8_callback_info->args);
-    ZVAL_COPY_VALUE(&php_v8_callback_info->gc_data[1], &php_v8_callback_info->retval);
+    ZVAL_COPY_VALUE(&php_v8_callback_info->gc_data[1], &php_v8_callback_info->php_v8_return_value->this_ptr);
 
     *table = php_v8_callback_info->gc_data;
     *n     = php_v8_callback_info->gc_data_count;
@@ -93,8 +90,12 @@ void php_v8_callback_info_free(zend_object *object) {
         delete php_v8_callback_info->holder_obj;
     }
 
-    if (!Z_ISUNDEF(php_v8_callback_info->retval)) {
-        zval_ptr_dtor(&php_v8_callback_info->retval);
+    if (php_v8_callback_info->php_v8_return_value) {
+        if (!Z_ISUNDEF(php_v8_callback_info->php_v8_return_value->this_ptr)) {
+            zval_ptr_dtor(&php_v8_callback_info->php_v8_return_value->this_ptr);
+        }
+
+        php_v8_callback_info->php_v8_return_value = NULL;
     }
 
     if (!Z_ISUNDEF(php_v8_callback_info->args)) {
@@ -186,7 +187,7 @@ static PHP_METHOD(V8CallbackInfo, GetReturnValue) {
     PHP_V8_CALLBACK_INFO_FETCH_WITH_CHECK(getThis(), php_v8_callback_info);
     PHP_V8_V8_CALLBACK_INFO_CHECK_IN_CONTEXT(php_v8_callback_info);
 
-    RETVAL_ZVAL(&php_v8_callback_info->retval, 1, 0);
+    RETVAL_ZVAL(&php_v8_callback_info->php_v8_return_value->this_ptr, 1, 0);
 }
 
 

src/php_v8_callback_info.h

@@ -17,6 +17,7 @@
 
 typedef struct _php_v8_callback_info_t php_v8_callback_info_t;
 
+#include "php_v8_return_value.h"
 #include "php_v8_exceptions.h"
 #include "php_v8_context.h"
 #include "php_v8_isolate.h"
@@ -34,7 +35,7 @@ extern zend_class_entry* php_v8_callback_info_class_entry;
 
 
 extern php_v8_callback_info_t * php_v8_callback_info_fetch_object(zend_object *obj);
-extern void php_v8_callback_info_invalidate(zval *val);
+extern void php_v8_callback_info_invalidate(php_v8_callback_info_t *php_v8_callback_info);
 
 #define PHP_V8_CALLBACK_INFO_FETCH(zv) php_v8_callback_info_fetch_object(Z_OBJ_P(zv))
 #define PHP_V8_CALLBACK_INFO_FETCH_INTO(pzval, into) php_v8_callback_info_t *(into) = PHP_V8_CALLBACK_INFO_FETCH((pzval));
@@ -51,7 +52,7 @@ extern void php_v8_callback_info_invalidate(zval *val);
 
 // TODO: suggest better naming
 #define PHP_V8_V8_CALLBACK_INFO_CHECK_IN_CONTEXT(value) \
-    if ((value)->php_v8_isolate == NULL) { \
+    if ((value)->php_v8_return_value == NULL || PHP_V8_RETVAL_ACCEPTS_INVALID == (value)->php_v8_return_value->accepts) { \
         PHP_V8_THROW_EXCEPTION("Attempt to use callback info object out of callback context"); \
         return; \
     }
@@ -69,7 +70,7 @@ struct _php_v8_callback_info_t {
     v8::Persistent<v8::Object> *holder_obj;
     bool is_construct_call;
 
-    zval retval;
+    php_v8_return_value_t *php_v8_return_value;
     zval args;
 
     zval *gc_data;

src/php_v8_callbacks.cc

@@ -419,16 +419,21 @@ void php_v8_callback_call_from_bucket_with_zargs(size_t index, v8::Local<v8::Val
 template<class T>
 void php_v8_callback_call_from_bucket_with_zargs(size_t index, const T &info, zval *args) {
     zval callback_info;
-
+    php_v8_callback_info_t *php_v8_callback_info;
     // Wrap callback info
-    php_v8_callback_info_create_from_info(&callback_info, info);
+    php_v8_callback_info = php_v8_callback_info_create_from_info(&callback_info, info);
+
+    if (!php_v8_callback_info) {
+        return;
+    }
+
     add_next_index_zval(args, &callback_info);
 
     php_v8_callback_call_from_bucket_with_zargs(index, info.Data(), args, NULL);
 
-    php_v8_callback_set_retval_from_callback_info(info.GetReturnValue(), PHP_V8_RETURN_VALUE_FETCH(&PHP_V8_CALLBACK_INFO_FETCH(&callback_info)->retval));
+    php_v8_callback_set_retval_from_callback_info(info.GetReturnValue(), php_v8_callback_info->php_v8_return_value);
 
-    php_v8_callback_info_invalidate(&callback_info);
+    php_v8_callback_info_invalidate(php_v8_callback_info);
 }
 
 

src/php_v8_function_callback_info.cc

@@ -27,13 +27,14 @@ zend_class_entry* php_v8_function_callback_info_class_entry;
 #define this_ce php_v8_function_callback_info_class_entry
 
 
-void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::FunctionCallbackInfo<v8::Value> &args) {
+php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::FunctionCallbackInfo<v8::Value> &args) {
+    zval retval;
     v8::Isolate *isolate = args.GetIsolate();
     v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
     if (context.IsEmpty()) {
         PHP_V8_THROW_EXCEPTION("Internal exception: no calling context found");
-        return;
+        return NULL;
     }
 
     object_init_ex(this_ptr, this_ce);
@@ -42,15 +43,15 @@ void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::FunctionCal
     php_v8_callback_info->php_v8_isolate = PHP_V8_ISOLATE_FETCH_REFERENCE(isolate);
     php_v8_callback_info->php_v8_context = php_v8_context_get_reference(context);
 
-    PHP_V8_COPY_ISOLATE_OBJECT_HANDLE(php_v8_callback_info->php_v8_isolate, php_v8_callback_info);
-
     php_v8_callback_info->this_obj->Reset(isolate, args.This());
     php_v8_callback_info->holder_obj->Reset(isolate, args.Holder());
 
-    php_v8_return_value_create_from_return_value(&php_v8_callback_info->retval,
-                                                 php_v8_callback_info->php_v8_isolate,
-                                                 php_v8_callback_info->php_v8_context,
-                                                 PHP_V8_RETVAL_ACCEPTS_ANY);
+    php_v8_callback_info->php_v8_return_value = php_v8_return_value_create_from_return_value(
+            &retval,
+            php_v8_callback_info->php_v8_isolate,
+            php_v8_callback_info->php_v8_context,
+            PHP_V8_RETVAL_ACCEPTS_ANY
+    );
 
     /* function callback specific part */
     php_v8_callback_info->length = args.Length();
@@ -62,6 +63,8 @@ void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::FunctionCal
     }
 
     php_v8_callback_info->is_construct_call = args.IsConstructCall();
+
+    return php_v8_callback_info;
 }
 
 

src/php_v8_function_callback_info.h

@@ -15,6 +15,7 @@
 #ifndef PHP_V8_FUNCTION_CALLBACK_INFO_H
 #define PHP_V8_FUNCTION_CALLBACK_INFO_H
 
+#include "php_v8_callback_info.h"
 #include <v8.h>
 
 extern "C" {
@@ -27,7 +28,7 @@ extern "C" {
 
 extern zend_class_entry* php_v8_function_callback_info_class_entry;
 
-extern void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::FunctionCallbackInfo<v8::Value>&args);
+extern php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::FunctionCallbackInfo<v8::Value>&args);
 
 
 PHP_MINIT_FUNCTION (php_v8_function_callback_info);

src/php_v8_property_callback_info.cc

@@ -26,37 +26,38 @@ zend_class_entry *php_v8_property_callback_info_class_entry;
 
 
 template<class T>
-void php_v8_callback_info_create_from_info_meta(zval *this_ptr, const v8::PropertyCallbackInfo<T> &info, int accepts);
+php_v8_callback_info_t *php_v8_callback_info_create_from_info_meta(zval *this_ptr, const v8::PropertyCallbackInfo<T> &info, int accepts);
 
 
-void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Value> &info) {
-    php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_ANY);
+php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Value> &info) {
+    return php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_ANY);
 }
 
-void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Array> &info) {
-    php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_ARRAY);
+php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Array> &info) {
+    return php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_ARRAY);
 }
 
-void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Integer> &info) {
-    php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_INTEGER);
+php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Integer> &info) {
+    return php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_INTEGER);
 }
 
-void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Boolean> &info) {
-    php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_BOOLEAN);
+php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Boolean> &info) {
+    return php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_BOOLEAN);
 }
 
-void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<void> &info) {
-    php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_VOID);
+php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<void> &info) {
+    return php_v8_callback_info_create_from_info_meta(this_ptr, info, PHP_V8_RETVAL_ACCEPTS_VOID);
 }
 
 template<class T>
-void php_v8_callback_info_create_from_info_meta(zval *this_ptr, const v8::PropertyCallbackInfo<T> &info, int accepts) {
+php_v8_callback_info_t *php_v8_callback_info_create_from_info_meta(zval *this_ptr, const v8::PropertyCallbackInfo<T> &info, int accepts) {
+    zval retval;
     v8::Isolate *isolate = info.GetIsolate();
     v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
     if (context.IsEmpty()) {
         PHP_V8_THROW_EXCEPTION("Internal exception: no calling context found");
-        return;
+        return NULL;
     }
 
     object_init_ex(this_ptr, this_ce);
@@ -65,15 +66,17 @@ void php_v8_callback_info_create_from_info_meta(zval *this_ptr, const v8::Proper
     php_v8_callback_info->php_v8_isolate = PHP_V8_ISOLATE_FETCH_REFERENCE(isolate);
     php_v8_callback_info->php_v8_context = php_v8_context_get_reference(context);
 
-    PHP_V8_COPY_ISOLATE_OBJECT_HANDLE(php_v8_callback_info->php_v8_isolate, php_v8_callback_info);
-
     php_v8_callback_info->this_obj->Reset(isolate, info.This());
     php_v8_callback_info->holder_obj->Reset(isolate, info.Holder());
 
-    php_v8_return_value_create_from_return_value(&php_v8_callback_info->retval,
-                                                 php_v8_callback_info->php_v8_isolate,
-                                                 php_v8_callback_info->php_v8_context,
-                                                 accepts);
+    php_v8_callback_info->php_v8_return_value = php_v8_return_value_create_from_return_value(
+            &retval,
+            php_v8_callback_info->php_v8_isolate,
+            php_v8_callback_info->php_v8_context,
+            accepts
+    );
+
+    return php_v8_callback_info;
 }
 
 static const zend_function_entry php_v8_property_callback_info_methods[] = {

src/php_v8_property_callback_info.h

@@ -15,6 +15,7 @@
 #ifndef PHP_V8_PROPERTY_CALLBACK_INFO_H
 #define PHP_V8_PROPERTY_CALLBACK_INFO_H
 
+#include "php_v8_callback_info.h"
 #include <v8.h>
 
 extern "C" {
@@ -27,11 +28,11 @@ extern "C" {
 
 extern zend_class_entry* php_v8_property_callback_info_class_entry;
 
-extern void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Value> &info);
-extern void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Array> &info);
-extern void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Integer> &info);
-extern void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Boolean> &info);
-extern void php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<void> &info);
+extern php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Value> &info);
+extern php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Array> &info);
+extern php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Integer> &info);
+extern php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<v8::Boolean> &info);
+extern php_v8_callback_info_t *php_v8_callback_info_create_from_info(zval *this_ptr, const v8::PropertyCallbackInfo<void> &info);
 
 
 PHP_MINIT_FUNCTION (php_v8_property_callback_info);

src/php_v8_return_value.cc

@@ -65,6 +65,10 @@ static void php_v8_return_value_free(zend_object *object) {
         zval_ptr_dtor(&php_v8_return_value->value.php_v8_value_zv);
     }
 
+    if (!Z_ISUNDEF(php_v8_return_value->this_ptr)) {
+        zval_ptr_dtor(&php_v8_return_value->this_ptr);
+    }
+
     if (php_v8_return_value->gc_data) {
         efree(php_v8_return_value->gc_data);
     }
@@ -87,19 +91,21 @@ static zend_object * php_v8_return_value_ctor(zend_class_entry *ce) {
 }
 
 
-void php_v8_return_value_create_from_return_value(zval *this_ptr, php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, int accepts) {
+php_v8_return_value_t *php_v8_return_value_create_from_return_value(zval *this_ptr, php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, int accepts) {
     object_init_ex(this_ptr, this_ce);
 
     PHP_V8_RETURN_VALUE_FETCH_INTO(this_ptr, php_v8_return_value);
 
     php_v8_return_value->php_v8_isolate = php_v8_isolate;
     php_v8_return_value->php_v8_context = php_v8_context;
     php_v8_return_value->accepts = accepts;
-}
 
-void php_v8_return_value_mark_expired(zval *this_ptr) {
-    PHP_V8_RETURN_VALUE_FETCH_INTO(this_ptr, php_v8_return_value);
+    ZVAL_COPY_VALUE(&php_v8_return_value->this_ptr, this_ptr);
+
+    return php_v8_return_value;
+}
 
+void php_v8_return_value_mark_expired(php_v8_return_value_t *php_v8_return_value) {
     php_v8_return_value->accepts = PHP_V8_RETVAL_ACCEPTS_INVALID;
 }
 

src/php_v8_return_value.h

@@ -32,9 +32,9 @@ extern "C" {
 
 extern zend_class_entry *php_v8_return_value_class_entry;
 
-extern void php_v8_return_value_create_from_return_value(zval *this_ptr, php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, int accepts);
-extern void php_v8_return_value_mark_expired(zval *this_ptr);
-extern php_v8_return_value_t * php_v8_return_value_fetch_object(zend_object *obj);
+extern php_v8_return_value_t *php_v8_return_value_create_from_return_value(zval *this_ptr, php_v8_isolate_t *php_v8_isolate, php_v8_context_t *php_v8_context, int accepts);
+extern void php_v8_return_value_mark_expired(php_v8_return_value_t *php_v8_return_value);
+extern php_v8_return_value_t *php_v8_return_value_fetch_object(zend_object *obj);
 
 
 #define PHP_V8_RETURN_VALUE_FETCH(zv) php_v8_return_value_fetch_object(Z_OBJ_P(zv))
@@ -128,6 +128,7 @@ struct _php_v8_return_value_t {
     zval *gc_data;
     int   gc_data_count;
 
+    zval this_ptr;
     zend_object std;
 };