Stop JS execution on PHP exceptions, refs #144

tests/exception_propagation_2.phpt

@@ -1,6 +1,7 @@
 --TEST--
 Test V8::executeString() : Exception propagation test 2
 --SKIPIF--
+SKIP needs discussion, see issue #144
 <?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
 --FILE--
 <?php

tests/php_exceptions_001.phpt

@@ -0,0 +1,50 @@
+--TEST--
+Test V8::executeString() : PHP Exception handling (repeated)
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+    function throwException() {
+	throw new \Exception("Test-Exception");
+    }
+}
+
+$v8 = new V8Js();
+$v8->foo = new \Foo();
+
+$JS = <<< EOT
+try {
+    PHP.foo.throwException();
+    // the exception should abort further execution,
+    // hence the print must not pop up
+    print("after throwException\\n");
+} catch(e) {
+    // JS should not catch in default mode
+    print("JS caught exception");
+}
+EOT;
+
+for($i = 0; $i < 5; $i ++) {
+    var_dump($i);
+    try {
+        $v8->executeString($JS);
+    } catch (Exception $e) {
+        var_dump($e->getMessage());
+    }
+}
+?>
+===EOF===
+--EXPECTF--
+int(0)
+string(14) "Test-Exception"
+int(1)
+string(14) "Test-Exception"
+int(2)
+string(14) "Test-Exception"
+int(3)
+string(14) "Test-Exception"
+int(4)
+string(14) "Test-Exception"
+===EOF===

tests/php_exceptions_002.phpt

@@ -0,0 +1,67 @@
+--TEST--
+Test V8::executeString() : PHP Exception handling (multi-level)
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+    function throwException() {
+	throw new \Exception("Test-Exception");
+    }
+
+    function recurse($i) {
+        echo "recurse[$i] ...\n";
+        global $work;
+        $work($i);
+    }
+}
+
+$v8 = new V8Js();
+$v8->foo = new \Foo();
+
+$work = $v8->executeString(<<<EOT
+var work = function(level) {
+  if(level--) {
+    PHP.foo.recurse(level);
+  }
+  else {
+    PHP.foo.throwException();
+  }
+};
+work;
+EOT
+);
+
+for($i = 0; $i < 5; $i ++) {
+    var_dump($i);
+    try {
+        $work($i);
+    } catch (Exception $e) {
+        var_dump($e->getMessage());
+    }
+}
+?>
+===EOF===
+--EXPECT--
+int(0)
+string(14) "Test-Exception"
+int(1)
+recurse[0] ...
+string(14) "Test-Exception"
+int(2)
+recurse[1] ...
+recurse[0] ...
+string(14) "Test-Exception"
+int(3)
+recurse[2] ...
+recurse[1] ...
+recurse[0] ...
+string(14) "Test-Exception"
+int(4)
+recurse[3] ...
+recurse[2] ...
+recurse[1] ...
+recurse[0] ...
+string(14) "Test-Exception"
+===EOF===

tests/php_exceptions_basic.phpt

@@ -0,0 +1,42 @@
+--TEST--
+Test V8::executeString() : PHP Exception handling (basic)
+--SKIPIF--
+<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
+--FILE--
+<?php
+
+class Foo {
+    function throwException() {
+	throw new \Exception("Test-Exception");
+    }
+}
+
+$v8 = new V8Js();
+$v8->foo = new \Foo();
+
+$JS = <<< EOT
+try {
+    PHP.foo.throwException();
+    // the exception should abort further execution,
+    // hence the print must not pop up
+    print("after throwException\\n");
+} catch(e) {
+    // JS should not catch in default mode
+    print("JS caught exception");
+}
+EOT;
+
+try {
+    $v8->executeString($JS);
+} catch (Exception $e) {
+    var_dump($e->getMessage());
+    var_dump($e->getFile());
+    var_dump($e->getLine());
+}
+?>
+===EOF===
+--EXPECTF--
+string(14) "Test-Exception"
+string(%d) "%sphp_exceptions_basic.php"
+int(5)
+===EOF===

v8js_methods.cc

@@ -26,20 +26,7 @@ extern "C" {
 V8JS_METHOD(exit) /* {{{ */
 {
 	v8::Isolate *isolate = info.GetIsolate();
-
+	v8js_terminate_execution(isolate);
-	/* Unfortunately just calling TerminateExecution on the isolate is not
-	 * enough, since v8 just marks the thread as "to be aborted" and doesn't
-	 * immediately do so.  Hence we enter an endless loop after signalling
-	 * termination, so we definitely don't execute JS code after the exit()
-	 * statement. */
-	v8::Locker locker(isolate);
-	v8::Isolate::Scope isolate_scope(isolate);
-	v8::HandleScope handle_scope(isolate);
-
-	v8::Local<v8::String> source = V8JS_STR("for(;;);");
-	v8::Local<v8::Script> script = v8::Script::Compile(source);
-	v8::V8::TerminateExecution(isolate);
-	script->Run();
 }
 /* }}} */
 

v8js_object_export.cc

@@ -134,11 +134,15 @@ static void v8js_call_php_func(zval *value, zend_class_entry *ce, zend_function
 		isolate->Enter();
 	}
 	zend_catch {
-		v8::V8::TerminateExecution(isolate);
+		v8js_terminate_execution(isolate);
 		V8JSG(fatal_error_abort) = 1;
 	}
 	zend_end_try();
 
+	if(EG(exception)) {
+		v8js_terminate_execution(isolate);
+	}
+
 failure:
 	/* Cleanup */
 	if (argc) {

v8js_timer.cc

@@ -55,7 +55,7 @@ static void v8js_timer_interrupt_handler(v8::Isolate *isolate, void *data) { /*
 
 		if (timer_ctx->memory_limit > 0 && hs.used_heap_size() > timer_ctx->memory_limit) {
 			timer_ctx->killed = true;
-			v8js_terminate_execution(c TSRMLS_CC);
+			v8::V8::TerminateExecution(c->isolate);
 			c->memory_limit_hit = true;
 		}
 	}
@@ -80,7 +80,7 @@ void v8js_timer_thread(TSRMLS_D) /* {{{ */
 			}
 			else if(timer_ctx->time_limit > 0 && now > timer_ctx->time_point) {
 				timer_ctx->killed = true;
-				v8js_terminate_execution(c TSRMLS_CC);
+				v8::V8::TerminateExecution(c->isolate);
 				c->time_limit_hit = true;
 			}
 			else if (timer_ctx->memory_limit > 0) {

v8js_v8.cc

@@ -199,10 +199,21 @@ void v8js_v8_call(v8js_ctx *c, zval **return_value,
 }
 /* }}} */
 
-void v8js_terminate_execution(v8js_ctx *c TSRMLS_DC) /* {{{ */
+void v8js_terminate_execution(v8::Isolate *isolate) /* {{{ */
 {
-	// Forcefully terminate the current thread of V8 execution in the isolate
+	/* Unfortunately just calling TerminateExecution on the isolate is not
-	v8::V8::TerminateExecution(c->isolate);
+	 * enough, since v8 just marks the thread as "to be aborted" and doesn't
+	 * immediately do so.  Hence we enter an endless loop after signalling
+	 * termination, so we definitely don't execute JS code after the exit()
+	 * statement. */
+	v8::Locker locker(isolate);
+	v8::Isolate::Scope isolate_scope(isolate);
+	v8::HandleScope handle_scope(isolate);
+
+	v8::Local<v8::String> source = V8JS_STR("for(;;);");
+	v8::Local<v8::Script> script = v8::Script::Compile(source);
+	v8::V8::TerminateExecution(isolate);
+	script->Run();
 }
 /* }}} */
 

v8js_v8.h

@@ -45,7 +45,7 @@ void v8js_v8_init(TSRMLS_D);
 void v8js_v8_call(v8js_ctx *c, zval **return_value,
 				  long flags, long time_limit, long memory_limit,
 				  std::function< v8::Local<v8::Value>(v8::Isolate *) >& v8_call TSRMLS_DC);
-void v8js_terminate_execution(v8js_ctx *c TSRMLS_DC);
+void v8js_terminate_execution(v8::Isolate *isolate);
 
 /* Fetch V8 object properties */
 int v8js_get_properties_hash(v8::Handle<v8::Value> jsValue, HashTable *retval, int flags, v8::Isolate *isolate TSRMLS_DC);