Merge branch 'php7' of https://github.com/phpv8/v8js into php8

tests/issue_472_basic.phpt

@@ -0,0 +1,30 @@
+--TEST--
+Test V8::executeString() : Issue #472 Destroy V8Js object which V8 isolate entered
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+class myjs extends \V8Js
+{
+   public function bosh()
+   {
+      $GLOBALS['v8test'] = null;
+	  unset($GLOBALS['v8test']);
+	}
+}
+
+$GLOBALS['v8test'] = new myjs('myjs');
+$ret = $GLOBALS['v8test']->executeString('
+	(() => {
+		myjs.bosh()
+	})
+');
+
+$ret();
+var_dump($ret);
+?>
+===EOF===
+--EXPECTF--
+object(V8Function)#%d (0) {
+}
+===EOF===

v8js_class.h

@@ -83,6 +83,10 @@ static inline struct v8js_ctx *v8js_ctx_fetch_object(zend_object *obj) {
 	return (struct v8js_ctx *)((char *)obj - XtOffsetOf(struct v8js_ctx, std));
 }
 
+static inline zend_object *v8js_ctx_to_zend_object(struct v8js_ctx *ctx) {
+	return (zend_object *)((char *)ctx + XtOffsetOf(struct v8js_ctx, std));
+}
+
 #define Z_V8JS_CTX_OBJ_P(zv) v8js_ctx_fetch_object(Z_OBJ_P(zv));
 #define Z_V8JS_CTX_OBJ(zv) v8js_ctx_fetch_object(zv);
 

v8js_v8.cc

@@ -120,135 +120,151 @@ void v8js_v8_call(v8js_ctx *c, zval **return_value,
 {
 	char *tz = NULL;
 
-	V8JS_CTX_PROLOGUE(c);
+	// hold extra reference on v8 instance as long as we call into V8 (issue #472)
+	zend_object *obj = v8js_ctx_to_zend_object(c);
+	zval zv_v8inst;
+	ZVAL_OBJ(&zv_v8inst, obj);
+	Z_ADDREF_P(&zv_v8inst);
 
-	V8JSG(timer_mutex).lock();
-	c->time_limit_hit = false;
-	c->memory_limit_hit = false;
-	V8JSG(timer_mutex).unlock();
+	{
+		V8JS_CTX_PROLOGUE(c);
 
-	/* Catch JS exceptions */
-	v8::TryCatch try_catch(isolate);
+		V8JSG(timer_mutex).lock();
+		c->time_limit_hit = false;
+		c->memory_limit_hit = false;
+		V8JSG(timer_mutex).unlock();
 
-	/* Set flags for runtime use */
-	c->flags = flags;
+		/* Catch JS exceptions */
+		v8::TryCatch try_catch(isolate);
 
-	/* Check if timezone has been changed and notify V8 */
-	tz = getenv("TZ");
+		/* Set flags for runtime use */
+		c->flags = flags;
 
-	if (tz != NULL) {
-		if (c->tz == NULL) {
-			c->tz = strdup(tz);
-		}
-		else if (strcmp(c->tz, tz) != 0) {
-			c->isolate->DateTimeConfigurationChangeNotification();
+		/* Check if timezone has been changed and notify V8 */
+		tz = getenv("TZ");
 
-			free(c->tz);
-			c->tz = strdup(tz);
-		}
-	}
+		if (tz != NULL) {
+			if (c->tz == NULL) {
+				c->tz = strdup(tz);
+			}
+			else if (strcmp(c->tz, tz) != 0) {
+				c->isolate->DateTimeConfigurationChangeNotification();
 
-	if (time_limit > 0 || memory_limit > 0) {
-		// If timer thread is not running then start it
-		if (!V8JSG(timer_thread)) {
-			// If not, start timer thread
-			V8JSG(timer_thread) = new std::thread(v8js_timer_thread, ZEND_MODULE_GLOBALS_BULK(v8js));
+				free(c->tz);
+				c->tz = strdup(tz);
+			}
 		}
-	}
 
-	/* Always pass the timer to the stack so there can be follow-up changes to
-	 * the time & memory limit. */
-	v8js_timer_push(time_limit, memory_limit, c);
+		if (time_limit > 0 || memory_limit > 0) {
+			// If timer thread is not running then start it
+			if (!V8JSG(timer_thread)) {
+				// If not, start timer thread
+				V8JSG(timer_thread) = new std::thread(v8js_timer_thread, ZEND_MODULE_GLOBALS_BULK(v8js));
+			}
+		}
 
-	/* Execute script */
-	c->in_execution++;
-	v8::MaybeLocal<v8::Value> result = v8_call(c->isolate);
-	c->in_execution--;
+		/* Always pass the timer to the stack so there can be follow-up changes to
+		 * the time & memory limit. */
+		v8js_timer_push(time_limit, memory_limit, c);
 
-	/* Pop our context from the stack and read (possibly updated) limits
-	 * into local variables. */
-	V8JSG(timer_mutex).lock();
-	v8js_timer_ctx *timer_ctx = V8JSG(timer_stack).front();
-	V8JSG(timer_stack).pop_front();
-	V8JSG(timer_mutex).unlock();
+		/* Execute script */
+		c->in_execution++;
+		v8::MaybeLocal<v8::Value> result = v8_call(c->isolate);
+		c->in_execution--;
 
-	time_limit = timer_ctx->time_limit;
-	memory_limit = timer_ctx->memory_limit;
+		/* Pop our context from the stack and read (possibly updated) limits
+		 * into local variables. */
+		V8JSG(timer_mutex).lock();
+		v8js_timer_ctx *timer_ctx = V8JSG(timer_stack).front();
+		V8JSG(timer_stack).pop_front();
+		V8JSG(timer_mutex).unlock();
 
-	efree(timer_ctx);
+		time_limit = timer_ctx->time_limit;
+		memory_limit = timer_ctx->memory_limit;
 
-	if(!V8JSG(fatal_error_abort)) {
-		char exception_string[64];
+		efree(timer_ctx);
 
-		if (c->time_limit_hit) {
-			// Execution has been terminated due to time limit
-			sprintf(exception_string, "Script time limit of %lu milliseconds exceeded", time_limit);
-			zend_throw_exception(php_ce_v8js_time_limit_exception, exception_string, 0);
-			return;
-		}
+		if(!V8JSG(fatal_error_abort)) {
+			char exception_string[64];
 
-		if (memory_limit && !c->memory_limit_hit) {
-			// Re-check memory limit (very short executions might never be hit by timer thread)
-			v8::HeapStatistics hs;
-			isolate->GetHeapStatistics(&hs);
+			if (c->time_limit_hit) {
+				// Execution has been terminated due to time limit
+				sprintf(exception_string, "Script time limit of %lu milliseconds exceeded", time_limit);
+				zend_throw_exception(php_ce_v8js_time_limit_exception, exception_string, 0);
+				zval_ptr_dtor(&zv_v8inst);
+				return;
+			}
 
-			if (hs.used_heap_size() > memory_limit) {
-				isolate->LowMemoryNotification();
+			if (memory_limit && !c->memory_limit_hit) {
+				// Re-check memory limit (very short executions might never be hit by timer thread)
+				v8::HeapStatistics hs;
 				isolate->GetHeapStatistics(&hs);
 
 				if (hs.used_heap_size() > memory_limit) {
-					c->memory_limit_hit = true;
+					isolate->LowMemoryNotification();
+					isolate->GetHeapStatistics(&hs);
+
+					if (hs.used_heap_size() > memory_limit) {
+						c->memory_limit_hit = true;
+					}
 				}
 			}
-		}
 
-		if (c->memory_limit_hit) {
-			// Execution has been terminated due to memory limit
-			sprintf(exception_string, "Script memory limit of %lu bytes exceeded", memory_limit);
-			zend_throw_exception(php_ce_v8js_memory_limit_exception, exception_string, 0);
-			return;
-		}
-
-		if (!try_catch.CanContinue()) {
-			// At this point we can't re-throw the exception
-			return;
-		}
-
-		/* There was pending exception left from earlier executions -> throw to PHP */
-		if (Z_TYPE(c->pending_exception) == IS_OBJECT) {
-			zend_throw_exception_object(&c->pending_exception);
-			ZVAL_NULL(&c->pending_exception);
-		}
+			if (c->memory_limit_hit) {
+				// Execution has been terminated due to memory limit
+				sprintf(exception_string, "Script memory limit of %lu bytes exceeded", memory_limit);
+				zend_throw_exception(php_ce_v8js_memory_limit_exception, exception_string, 0);
+				zval_ptr_dtor(&zv_v8inst);
+				return;
+			}
 
-		/* Handle runtime JS exceptions */
-		if (try_catch.HasCaught()) {
+			if (!try_catch.CanContinue()) {
+				// At this point we can't re-throw the exception
+				zval_ptr_dtor(&zv_v8inst);
+				return;
+			}
 
-			/* Pending exceptions are set only in outer caller, inner caller exceptions are always rethrown */
-			if (c->in_execution < 1) {
+			/* There was pending exception left from earlier executions -> throw to PHP */
+			if (Z_TYPE(c->pending_exception) == IS_OBJECT) {
+				zend_throw_exception_object(&c->pending_exception);
+				ZVAL_NULL(&c->pending_exception);
+			}
 
-				/* Report immediately if report_uncaught is true */
-				if (c->report_uncaught) {
-					v8js_throw_script_exception(c->isolate, &try_catch);
-					return;
+			/* Handle runtime JS exceptions */
+			if (try_catch.HasCaught()) {
+
+				/* Pending exceptions are set only in outer caller, inner caller exceptions are always rethrown */
+				if (c->in_execution < 1) {
+
+					/* Report immediately if report_uncaught is true */
+					if (c->report_uncaught) {
+						v8js_throw_script_exception(c->isolate, &try_catch);
+						zval_ptr_dtor(&zv_v8inst);
+						return;
+					}
+
+					/* Exception thrown from JS, preserve it for future execution */
+					if (result.IsEmpty()) {
+						v8js_create_script_exception(&c->pending_exception, c->isolate, &try_catch);
+						zval_ptr_dtor(&zv_v8inst);
+						return;
+					}
 				}
 
-				/* Exception thrown from JS, preserve it for future execution */
-				if (result.IsEmpty()) {
-					v8js_create_script_exception(&c->pending_exception, c->isolate, &try_catch);
-					return;
-				}
+				/* Rethrow back to JS */
+				try_catch.ReThrow();
+				zval_ptr_dtor(&zv_v8inst);
+				return;
 			}
 
-			/* Rethrow back to JS */
-			try_catch.ReThrow();
-			return;
-		}
-
-		/* Convert V8 value to PHP value */
-		if (return_value && !result.IsEmpty()) {
-			v8js_to_zval(result.ToLocalChecked(), *return_value, flags, c->isolate);
+			/* Convert V8 value to PHP value */
+			if (return_value && !result.IsEmpty()) {
+				v8js_to_zval(result.ToLocalChecked(), *return_value, flags, c->isolate);
+			}
 		}
 	}
+
+	zval_ptr_dtor(&zv_v8inst);
 }
 /* }}} */
 

v8js_v8object_class.cc

@@ -276,6 +276,14 @@ static HashTable *v8js_v8object_get_properties(zend_object *object) /* {{{ */
 }
 /* }}} */
 
+static HashTable *v8js_v8object_get_gc(zend_object *object, zval **table, int *n) /* {{{ */
+{
+	*table = NULL;
+	*n = 0;
+	return NULL;
+}
+/* }}} */
+
 static HashTable *v8js_v8object_get_debug_info(zend_object *object, int *is_temp) /* {{{ */
 {
 	*is_temp = 0;
@@ -456,118 +464,6 @@ static zend_function *v8js_v8object_get_method(zend_object **object_ptr, zend_st
 }
 /* }}} */
 
-static int v8js_v8object_call_method(zend_string *method, zend_object *object, INTERNAL_FUNCTION_PARAMETERS) /* {{{ */
-{
-	zval *argv = NULL;
-	int argc = ZEND_NUM_ARGS();
-
-	v8js_v8object *obj = v8js_v8object_fetch_object(object);
-
-	if (!obj->ctx)
-	{
-		zend_throw_exception(php_ce_v8js_exception,
-							 "Can't access V8Object after V8Js instance is destroyed!", 0);
-		return FAILURE;
-	}
-
-	if (obj->v8obj.IsEmpty())
-	{
-		return FAILURE;
-	}
-
-	if (ZSTR_LEN(method) > std::numeric_limits<int>::max())
-	{
-		zend_throw_exception(php_ce_v8js_exception,
-							 "Method name length exceeds maximum supported length", 0);
-		return FAILURE;
-	}
-
-	if (argc > 0)
-	{
-		argv = (zval *)safe_emalloc(sizeof(zval), argc, 0);
-		zend_get_parameters_array_ex(argc, argv);
-	}
-
-	/* std::function relies on its dtor to be executed, otherwise it leaks
-	 * some memory on bailout. */
-	{
-		std::function<v8::MaybeLocal<v8::Value>(v8::Isolate *)> v8_call = [obj, method, argc, argv, object, &return_value](v8::Isolate *isolate)
-		{
-			int i = 0;
-
-			v8::Local<v8::Context> v8_context = isolate->GetEnteredOrMicrotaskContext();
-			v8::Local<v8::String> method_name = V8JS_SYML(ZSTR_VAL(method), static_cast<int>(ZSTR_LEN(method)));
-			v8::Local<v8::Object> v8obj = v8::Local<v8::Value>::New(isolate, obj->v8obj)->ToObject(v8_context).ToLocalChecked();
-			v8::Local<v8::Object> thisObj;
-			v8::Local<v8::Function> cb;
-
-			if (method_name->Equals(v8_context, V8JS_SYM(V8JS_V8_INVOKE_FUNC_NAME)).FromMaybe(false))
-			{
-				cb = v8::Local<v8::Function>::Cast(v8obj);
-			}
-			else
-			{
-				v8::Local<v8::Value> slot;
-
-				if (!v8obj->Get(v8_context, method_name).ToLocal(&slot))
-				{
-					return v8::MaybeLocal<v8::Value>();
-				}
-
-				cb = v8::Local<v8::Function>::Cast(slot);
-			}
-
-			// If a method is invoked on V8Object, then set the object itself as
-			// "this" on JS side.  Otherwise fall back to global object.
-			if (obj->std.ce == php_ce_v8object)
-			{
-				thisObj = v8obj;
-			}
-			else
-			{
-				thisObj = V8JS_GLOBAL(isolate);
-			}
-
-			v8::Local<v8::Value> *jsArgv = static_cast<v8::Local<v8::Value> *>(alloca(sizeof(v8::Local<v8::Value>) * argc));
-
-			for (i = 0; i < argc; i++)
-			{
-				new (&jsArgv[i]) v8::Local<v8::Value>;
-				jsArgv[i] = v8::Local<v8::Value>::New(isolate, zval_to_v8js(&argv[i], isolate));
-			}
-
-			v8::MaybeLocal<v8::Value> result = cb->Call(v8_context, thisObj, argc, jsArgv);
-
-			if (obj->std.ce == php_ce_v8object && !result.IsEmpty() && result.ToLocalChecked()->StrictEquals(thisObj))
-			{
-				/* JS code did "return this", retain object identity */
-				ZVAL_OBJ(return_value, object);
-				zval_copy_ctor(return_value);
-				result = v8::MaybeLocal<v8::Value>();
-			}
-
-			return result;
-		};
-
-		v8js_v8_call(obj->ctx, &return_value, obj->flags, obj->ctx->time_limit, obj->ctx->memory_limit, v8_call);
-	}
-
-	if (argc > 0)
-	{
-		efree(argv);
-	}
-
-	if (V8JSG(fatal_error_abort))
-	{
-		/* Check for fatal error marker possibly set by v8js_error_handler; just
-		 * rethrow the error since we're now out of V8. */
-		zend_bailout();
-	}
-
-	return SUCCESS;
-}
-/* }}} */
-
 static int v8js_v8object_get_closure(zend_object *object, zend_class_entry **ce_ptr, zend_function **fptr_ptr, zend_object **zobj_ptr, bool call) /* {{{ */
 {
 	zend_internal_function *invoke;
@@ -1023,6 +919,7 @@ PHP_MINIT_FUNCTION(v8js_v8object_class) /* {{{ */
 	v8js_v8object_handlers.unset_property = v8js_v8object_unset_property;
 	v8js_v8object_handlers.get_properties = v8js_v8object_get_properties;
 	v8js_v8object_handlers.get_method = v8js_v8object_get_method;
+	v8js_v8object_handlers.get_gc = v8js_v8object_get_gc;
 	v8js_v8object_handlers.get_debug_info = v8js_v8object_get_debug_info;
 	v8js_v8object_handlers.get_closure = v8js_v8object_get_closure;
 	v8js_v8object_handlers.offset = XtOffsetOf(struct v8js_v8object, std);