Merge pull request #253 from stesie/issue-250

Fix refcounting, use zval_ptr_dtor, closes #250

tests/issue_250_001.phpt

@@ -0,0 +1,75 @@
+--TEST--
+Test V8::executeString() : Issue #250 (early free of array)
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class TestObject {
+
+    private $data = [];
+    private $meta = [];
+
+    public function setTitle($title) {
+        $this->a->b->title = $title;
+    }
+
+    public function getData() {
+        return $this->data;
+    }
+
+    public function getMeta() {
+        return $this->meta;
+    }
+
+    public function setData($data=[]) {
+        $this->data = $data;
+    }
+
+    public function setMeta($meta) {
+        return;
+    }
+}
+
+$v8 = new V8Js("server");
+$code = <<< EOT
+    var v1 = server.response.getData();
+    var v2 = server.response.getMeta();
+
+    server.response.setData({});
+    server.response.setTitle("ouch");
+    server.response.setMeta({});
+EOT;
+
+$response = new TestObject();
+
+$v8->response = $response;
+
+try {
+    $result = $v8->executeString($code);
+    var_dump($v8->response);
+} catch (V8JsException $e) {
+    var_dump($e);
+}
+
+?>
+===EOF===
+--EXPECTF--
+Warning: Creating default object from empty value in %s%eissue_250_001.php on line 9
+object(TestObject)#%d (3) {
+  ["data":"TestObject":private]=>
+  object(V8Object)#%d (0) {
+  }
+  ["meta":"TestObject":private]=>
+  array(0) {
+  }
+  ["a"]=>
+  object(stdClass)#%d (1) {
+    ["b"]=>
+    object(stdClass)#%d (1) {
+      ["title"]=>
+      string(4) "ouch"
+    }
+  }
+}
+===EOF===

tests/issue_250_002.phpt

@@ -0,0 +1,44 @@
+--TEST--
+Test V8::executeString() : Issue #250 (early free of array)
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class TestObject {
+
+    var $data;
+
+    public function setData($data) {
+        $this->data = $data;
+    }
+
+    public function getData() {
+        return $this->data;
+    }
+
+}
+
+$v8 = new V8Js("server");
+
+$code = <<< EOT
+    server.response.setData({"new": true});
+EOT;
+
+$v8->response = new TestObject();
+
+try {
+    $result = $v8->executeString($code, null, \V8Js::FLAG_FORCE_ARRAY | \V8Js::FLAG_PROPAGATE_PHP_EXCEPTIONS);
+    var_dump($v8->response->getData());
+} catch (V8JsException $e) {
+    var_dump($e);
+}
+
+?>
+===EOF===
+--EXPECT--
+array(1) {
+  ["new"]=>
+  bool(true)
+}
+===EOF===

v8js_array_access.cc

@@ -70,7 +70,7 @@ void v8js_array_access_getter(uint32_t index, const v8::PropertyCallbackInfo<v8:
 
 	zval php_value = v8js_array_access_dispatch(object, "offsetGet", 1, index, zvalue TSRMLS_CC);
 	v8::Local<v8::Value> ret_value = zval_to_v8js(&php_value, isolate TSRMLS_CC);
-	zval_dtor(&php_value);
+	zval_ptr_dtor(&php_value);
 
 	info.GetReturnValue().Set(ret_value);
 }
@@ -95,7 +95,7 @@ void v8js_array_access_setter(uint32_t index, v8::Local<v8::Value> value,
 	}
 
 	zval php_value = v8js_array_access_dispatch(object, "offsetSet", 2, index, zvalue TSRMLS_CC);
-	zval_dtor(&php_value);
+	zval_ptr_dtor(&php_value);
 
 	/* simply pass back the value to tell we intercepted the call
 	 * as the offsetSet function returns void. */
@@ -103,7 +103,7 @@ void v8js_array_access_setter(uint32_t index, v8::Local<v8::Value> value,
 
 	/* if PHP wanted to hold on to this value, zend_call_function would
 	 * have bumped the refcount. */
-	zval_dtor(&zvalue);
+	zval_ptr_dtor(&zvalue);
 }
 /* }}} */
 
@@ -117,7 +117,7 @@ static int v8js_array_access_get_count_result(zend_object *object TSRMLS_DC) /*
 
 	if(Z_TYPE(php_value) != IS_LONG) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-numeric return value from count() method");
-		zval_dtor(&php_value);
+		zval_ptr_dtor(&php_value);
 		return 0;
 	}
 
@@ -135,7 +135,7 @@ static bool v8js_array_access_isset_p(zend_object *object, int index TSRMLS_DC)
 
 	if(Z_TYPE(php_value) != IS_TRUE && Z_TYPE(php_value) != IS_FALSE) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-boolean return value from offsetExists() method");
-		zval_dtor(&php_value);
+		zval_ptr_dtor(&php_value);
 		return false;
 	}
 

v8js_class.cc

@@ -88,9 +88,9 @@ static void v8js_free_storage(zend_object *object TSRMLS_DC) /* {{{ */
 
 	zend_object_std_dtor(&c->std TSRMLS_CC);
 
-	zval_dtor(&c->pending_exception);
-	zval_dtor(&c->module_normaliser);
-	zval_dtor(&c->module_loader);
+	zval_ptr_dtor(&c->pending_exception);
+	zval_ptr_dtor(&c->module_normaliser);
+	zval_ptr_dtor(&c->module_loader);
 
 	/* Delete PHP global object from JavaScript */
 	if (!c->context.IsEmpty()) {
@@ -150,7 +150,7 @@ static void v8js_free_storage(zend_object *object TSRMLS_DC) /* {{{ */
 		zend_object *object = it->first;
 		zval value;
 		ZVAL_OBJ(&value, object);
-		zval_dtor(&value);
+		zval_ptr_dtor(&value);
 		c->isolate->AdjustAmountOfExternalAllocatedMemory(-c->average_object_size);
 		it->second.Reset();
 	}
@@ -200,7 +200,7 @@ static void v8js_free_storage(zend_object *object TSRMLS_DC) /* {{{ */
 	c->modules_stack.~vector();
 	c->modules_base.~vector();
 
-	zval_dtor(&c->zval_snapshot_blob);
+	zval_ptr_dtor(&c->zval_snapshot_blob);
 }
 /* }}} */
 
@@ -769,7 +769,7 @@ static PHP_METHOD(V8Js, clearPendingException)
 	c = Z_V8JS_CTX_OBJ_P(getThis());
 
 	if (Z_TYPE(c->pending_exception) == IS_OBJECT) {
-		zval_dtor(&c->pending_exception);
+		zval_ptr_dtor(&c->pending_exception);
 		ZVAL_NULL(&c->pending_exception);
 	}
 }

v8js_methods.cc

@@ -259,8 +259,8 @@ V8JS_METHOD(require)
 		}
 		zend_end_try();
 
-		zval_dtor(&params[0]);
-		zval_dtor(&params[1]);
+		zval_ptr_dtor(&params[0]);
+		zval_ptr_dtor(&params[1]);
 
 		if(call_result == FAILURE) {
 			return;
@@ -275,7 +275,7 @@ V8JS_METHOD(require)
 		}
 
 		if (Z_TYPE(normaliser_result) != IS_ARRAY) {
-			zval_dtor(&normaliser_result);
+			zval_ptr_dtor(&normaliser_result);
 			info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module normaliser didn't return an array")));
 			return;
 		}
@@ -284,7 +284,7 @@ V8JS_METHOD(require)
 		int num_elements = zend_hash_num_elements(ht);
 
 		if(num_elements != 2) {
-			zval_dtor(&normaliser_result);
+			zval_ptr_dtor(&normaliser_result);
 			info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module normaliser expected to return array of 2 strings")));
 			return;
 		}
@@ -309,7 +309,7 @@ V8JS_METHOD(require)
 		}
 		ZEND_HASH_FOREACH_END();
 
-		zval_dtor(&normaliser_result);
+		zval_ptr_dtor(&normaliser_result);
 	}
 
 	char *normalised_module_id = (char *)emalloc(strlen(normalised_path)+1+strlen(module_name)+1);
@@ -377,7 +377,7 @@ V8JS_METHOD(require)
 		info.GetReturnValue().Set(isolate->ThrowException(V8JS_SYM("Module loader callback failed")));
 	}
 
-	zval_dtor(&params[0]);
+	zval_ptr_dtor(&params[0]);
 
 	if (call_result == FAILURE) {
 		efree(normalised_module_id);

v8js_object_export.cc

@@ -81,7 +81,7 @@ static void v8js_call_php_func(zend_object *object, zend_function *method_ptr, v
 		}
 		efree(error);
 		info.GetReturnValue().Set(return_value);
-		zval_dtor(&fname);
+		zval_ptr_dtor(&fname);
 		return;
 	}
 
@@ -138,7 +138,7 @@ failure:
 	/* Cleanup */
 	if (argc) {
 		for (i = 0; i < fci.param_count; i++) {
-			zval_dtor(&fci.params[i]);
+			zval_ptr_dtor(&fci.params[i]);
 		}
 		efree(fci.params);
 	}
@@ -159,8 +159,8 @@ failure:
 		return_value = zval_to_v8js(&retval, isolate TSRMLS_CC);
 	}
 
-	zval_dtor(&retval);
-	zval_dtor(&fname);
+	zval_ptr_dtor(&retval);
+	zval_ptr_dtor(&fname);
 
 	info.GetReturnValue().Set(return_value);
 }
@@ -264,7 +264,7 @@ static void v8js_weak_object_callback(const v8::WeakCallbackInfo<zend_object> &d
 	zend_object *object = data.GetParameter();
 	zval value;
 	ZVAL_OBJ(&value, object);
-	zval_dtor(&value);
+	zval_ptr_dtor(&value);
 
 	v8js_ctx *ctx = (v8js_ctx *) isolate->GetData(0);
 
@@ -643,7 +643,7 @@ v8::Local<v8::Value> v8js_named_property_callback(v8::Local<v8::String> property
 				/* Okay, let's call __get. */
 				zend_call_method_with_1_params(&zobject, ce, &ce->__get, ZEND_GET_FUNC_NAME, &php_value, &zname);
 				ret_value = zval_to_v8js(&php_value, isolate TSRMLS_CC);
-				zval_dtor(&php_value);
+				zval_ptr_dtor(&php_value);
 			}
 
 		} else if (callback_type == V8JS_PROP_SETTER) {
@@ -701,7 +701,7 @@ v8::Local<v8::Value> v8js_named_property_callback(v8::Local<v8::String> property
 			ret_value = v8::Handle<v8::Value>();
 		}
 
-		zval_dtor(&zname);
+		zval_ptr_dtor(&zname);
 	}
 
 	zend_string_release(method_name);

v8js_v8.cc

@@ -282,7 +282,7 @@ int v8js_get_properties_hash(v8::Handle<v8::Value> jsValue, HashTable *retval, i
 			}
 			else {
 				if (v8js_to_zval(jsVal, &value, flags, isolate TSRMLS_CC) == FAILURE) {
-					zval_dtor(&value);
+					zval_ptr_dtor(&value);
 					return FAILURE;
 				}
 			}

v8js_v8object_class.cc

@@ -479,7 +479,7 @@ PHP_METHOD(V8Function, __wakeup)
 static void v8js_v8generator_free_storage(zend_object *object) /* {{{ */
 {
 	v8js_v8generator *c = v8js_v8generator_fetch_object(object);
-	zval_dtor(&c->value);
+	zval_ptr_dtor(&c->value);
 
 	v8js_v8object_free_storage(object);
 }
@@ -533,7 +533,7 @@ static void v8js_v8generator_next(v8js_v8generator *g) /* {{{ */
 			v8::Local<v8::Value> val = resultObj->Get(V8JS_STR("value"));
 			v8::Local<v8::Value> done = resultObj->Get(V8JS_STR("done"));
 
-			zval_dtor(&g->value);
+			zval_ptr_dtor(&g->value);
 			v8js_to_zval(val, &g->value, 0, isolate);
 
 			g->done = done->IsTrue();