Keep track of V8Object/V8Function instances

Disallow access to these once the V8Js object has been destroyed.

php_v8js_macros.h

@@ -183,6 +183,7 @@ void php_v8js_accessor_ctx_dtor(php_v8js_accessor_ctx * TSRMLS_DC);
 /* Register accessors into passed object */
 void php_v8js_register_accessors(std::vector<php_v8js_accessor_ctx*> *accessor_list, v8::Local<v8::FunctionTemplate>, zval *, v8::Isolate * TSRMLS_DC);
 
+struct php_v8js_object;
 
 /* {{{ Context container */
 struct php_v8js_ctx {
@@ -204,6 +205,8 @@ struct php_v8js_ctx {
   std::map<zval *, v8js_persistent_obj_t> weak_objects;
   std::map<v8js_tmpl_t *, v8js_persistent_obj_t> weak_closures;
 
+  std::list<php_v8js_object *> php_v8js_objects;
+
   std::vector<php_v8js_accessor_ctx *> accessor_list;
   char *tz;
 #ifdef ZTS
@@ -238,7 +241,7 @@ struct php_v8js_object {
 	zend_object std;
 	v8::Persistent<v8::Value> v8obj;
 	int flags;
-	v8::Isolate *isolate;
+	struct php_v8js_ctx *ctx;
 	HashTable *properties;
 };
 /* }}} */
@@ -260,7 +263,6 @@ ZEND_BEGIN_MODULE_GLOBALS(v8js)
   bool timer_stop;
 
   std::map<char *, v8::Handle<v8::Object> > modules_loaded;
-  std::list<v8::Isolate *> invalid_isolates;
 
   // fatal error unwinding
   bool fatal_error_abort;

tests/use_after_dispose.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Test V8::executeString() : Use after dispose
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+	function callMe($x) {
+		var_dump($x);
+		$this->x = $x;
+	}
+}
+
+$v8 = new V8Js();
+$v8->foo = $foo = new Foo();
+
+$JS = <<< EOT
+PHP.foo.callMe({ bla: 23 });
+
+EOT;
+
+$v8->executeString($JS, 'basic.js');
+unset($v8);
+
+try {
+	var_dump($foo->x);
+}
+catch(V8JsScriptException $e) {
+	var_dump($e->getMessage());
+}
+?>
+===EOF===
+--EXPECTF--
+object(V8Object)#%d (1) {
+  ["bla"]=>
+  int(23)
+}
+object(V8Object)#%d (0) {
+}
+string(55) "Can't access V8Object after V8Js instance is destroyed!"
+===EOF===

v8js.cc

@@ -130,7 +130,13 @@ static int php_v8js_v8_has_property(zval *object, zval *member, int has_set_exis
 	int retval = false;
 	php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC);
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return retval;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -189,7 +195,13 @@ static zval *php_v8js_v8_read_property(zval *object, zval *member, int type ZEND
 	zval *retval = NULL;
 	php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC);
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return retval;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -232,7 +244,13 @@ static void php_v8js_v8_write_property(zval *object, zval *member, zval *value Z
 {
 	php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC);
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -251,7 +269,13 @@ static void php_v8js_v8_unset_property(zval *object, zval *member ZEND_HASH_KEY_
 {
 	php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC);
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -327,7 +351,7 @@ static HashTable *php_v8js_v8_get_properties(zval *object TSRMLS_DC) /* {{{ */
 			/* the garbage collector is running, don't create more zvals */
 			return NULL;
 		}
-		if (obj->isolate == NULL) {
+		if (obj->ctx == NULL) {
 			/* Half-constructed object.  Shouldn't happen, but be safe. */
 			return NULL;
 		}
@@ -337,7 +361,13 @@ static HashTable *php_v8js_v8_get_properties(zval *object TSRMLS_DC) /* {{{ */
 		zend_hash_clean(obj->properties);
 	}
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return NULL;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -365,7 +395,13 @@ static zend_function *php_v8js_v8_get_method(zval **object_ptr, char *method, in
 	php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(*object_ptr TSRMLS_CC);
 	zend_function *f;
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return NULL;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -401,6 +437,12 @@ static int php_v8js_v8_call_method(char *method, INTERNAL_FUNCTION_PARAMETERS) /
 
 	obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC);
 
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return FAILURE;
+	}
+
 	if (obj->v8obj.IsEmpty()) {
 		zval_ptr_dtor(&object);
 		return FAILURE;
@@ -411,7 +453,7 @@ static int php_v8js_v8_call_method(char *method, INTERNAL_FUNCTION_PARAMETERS) /
 		zend_get_parameters_array_ex(argc, argv);
 	}
 
-	v8::Isolate *isolate = obj->isolate;
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -457,7 +499,13 @@ static int php_v8js_v8_get_closure(zval *object, zend_class_entry **ce_ptr, zend
 
 	php_v8js_object *obj = (php_v8js_object *) zend_object_store_get_object(object TSRMLS_CC);
 
-	v8::Isolate *isolate = obj->isolate;
+	if (!obj->ctx) {
+		zend_throw_exception(php_ce_v8js_script_exception,
+			"Can't access V8Object after V8Js instance is destroyed!", 0 TSRMLS_CC);
+		return FAILURE;
+	}
+
+	v8::Isolate *isolate = obj->ctx->isolate;
 	v8::Locker locker(isolate);
 	v8::Isolate::Scope isolate_scope(isolate);
 	v8::HandleScope local_scope(isolate);
@@ -497,16 +545,9 @@ static void php_v8js_v8_free_storage(void *object, zend_object_handle handle TSR
 
 	zend_object_std_dtor(&c->std TSRMLS_CC);
 
-	bool isolate_invalid = false;
-	for (std::list<v8::Isolate *>::iterator it = V8JSG(invalid_isolates).begin(); it != V8JSG(invalid_isolates).end(); ++it) {
-		if(*it == c->isolate) {
-			isolate_invalid = true;
-			break;
-		}
-	}
-
-	if(!isolate_invalid) {
+	if(c->ctx) {
 		c->v8obj.Reset();
+		c->ctx->php_v8js_objects.remove(c);
 	}
 
 	efree(object);
@@ -557,6 +598,13 @@ PHP_METHOD(V8Function,__construct)
 
 void php_v8js_create_v8(zval *res, v8::Handle<v8::Value> value, int flags, v8::Isolate *isolate TSRMLS_DC) /* {{{ */
 {
+#if PHP_V8_API_VERSION <= 3023008
+	/* Until V8 3.23.8 Isolate could only take one external pointer. */
+	php_v8js_ctx *ctx = (php_v8js_ctx *) isolate->GetData();
+#else
+	php_v8js_ctx *ctx = (php_v8js_ctx *) isolate->GetData(0);
+#endif
+
 	php_v8js_object *c;
 
 	object_init_ex(res, value->IsFunction() ? php_ce_v8_function : php_ce_v8_object);
@@ -565,8 +613,10 @@ void php_v8js_create_v8(zval *res, v8::Handle<v8::Value> value, int flags, v8::I
 
 	c->v8obj.Reset(isolate, value);
 	c->flags = flags;
-	c->isolate = isolate;
+	c->ctx = ctx;
 	c->properties = NULL;
+
+	ctx->php_v8js_objects.push_front(c);
 }
 /* }}} */
 
@@ -657,7 +707,13 @@ static void php_v8js_free_storage(void *object TSRMLS_DC) /* {{{ */
 	}
 	c->weak_closures.~map();
 
-	V8JSG(invalid_isolates).push_front(c->isolate);
+	for (std::list<php_v8js_object *>::iterator it = c->php_v8js_objects.begin();
+		 it != c->php_v8js_objects.end(); it ++) {
+		(*it)->v8obj.Reset();
+		(*it)->ctx = NULL;
+	}
+	c->php_v8js_objects.~list();
+
 	c->isolate->Dispose();
 
 	if(c->tz != NULL) {
@@ -699,6 +755,8 @@ static zend_object_value php_v8js_new(zend_class_entry *ce TSRMLS_DC) /* {{{ */
 	new(&c->weak_closures) std::map<v8js_tmpl_t *, v8js_persistent_obj_t>();
 	new(&c->weak_objects) std::map<zval *, v8js_persistent_obj_t>();
 
+	new(&c->php_v8js_objects) std::list<php_v8js_object *>();
+
 	retval.handle = zend_objects_store_put(c, NULL, (zend_objects_free_object_storage_t) php_v8js_free_storage, NULL TSRMLS_CC);
 	retval.handlers = &v8js_object_handlers;
 
@@ -853,8 +911,6 @@ static PHP_METHOD(V8Js, __construct)
 	c->memory_limit_hit = false;
 	c->module_loader = NULL;
 
-	V8JSG(invalid_isolates).remove(c->isolate);
-
 	/* Include extensions used by this context */
 	/* Note: Extensions registered with auto_enable do not need to be added separately like this. */
 	if (exts_arr)
@@ -1834,18 +1890,6 @@ static PHP_MSHUTDOWN_FUNCTION(v8js)
 }
 /* }}} */
 
-/* {{{ PHP_RINIT_FUNCTION
- */
-static PHP_RINIT_FUNCTION(v8js)
-{
-	// Clear list of invalid isolates which might remain from requests served
-	// earlier and the list cannot be cleared in shutdown handler (as it
-	// is called too early).
-	V8JSG(invalid_isolates).clear();
-	return SUCCESS;
-}
-/* }}} */
-
 /* {{{ PHP_RSHUTDOWN_FUNCTION
  */
 static PHP_RSHUTDOWN_FUNCTION(v8js)
@@ -1856,10 +1900,6 @@ static PHP_RSHUTDOWN_FUNCTION(v8js)
 		V8JSG(timer_thread)->join();
 	}
 
-	// We must *not* clear invalid_isolates here, since it is still possible
-	// that some php objects have references towards v8, which we mustn't use
-	// even after shutting down now.
-
 #if V8JS_DEBUG
 	v8::HeapStatistics stats;
 	v8::V8::GetHeapStatistics(&stats);
@@ -1913,7 +1953,6 @@ static PHP_GINIT_FUNCTION(v8js)
 	new(&v8js_globals->timer_mutex) std::mutex;
 	new(&v8js_globals->timer_stack) std::stack<php_v8js_timer_ctx *>;
 	new(&v8js_globals->modules_loaded) std::map<char *, v8::Handle<v8::Object>>;
-	new(&v8js_globals->invalid_isolates) std::list<v8::Isolate *>;
 
 	v8js_globals->fatal_error_abort = 0;
 	v8js_globals->error_num = 0;
@@ -1943,7 +1982,6 @@ static PHP_GSHUTDOWN_FUNCTION(v8js)
 	v8js_globals->timer_stack.~stack();
 	v8js_globals->timer_mutex.~mutex();
 	v8js_globals->modules_loaded.~map();
-	v8js_globals->invalid_isolates.~list();
 #endif
 }
 /* }}} */
@@ -1964,7 +2002,7 @@ zend_module_entry v8js_module_entry = {
 	v8js_functions,
 	PHP_MINIT(v8js),
 	PHP_MSHUTDOWN(v8js),
-	PHP_RINIT(v8js),
+	NULL,
 	PHP_RSHUTDOWN(v8js),
 	PHP_MINFO(v8js),
 	V8JS_VERSION,