Browse Source

defer bailout until std::function dtor

std::function allocates some heap memory, at least with some
implementations and expects the dtor to run.  Hence defer the
bailout until the dtor ran.
Stefan Siegl 9 years ago
parent
commit
a83c49266e
3 changed files with 147 additions and 128 deletions
  1. 15 5
      v8js_class.cc
  2. 92 93
      v8js_v8.cc
  3. 40 30
      v8js_v8object_class.cc

+ 15 - 5
v8js_class.cc

@@ -529,12 +529,22 @@ static void v8js_execute_script(zval *this_ptr, v8js_script *res, long flags, lo
 		memory_limit = c->memory_limit;
 		memory_limit = c->memory_limit;
 	}
 	}
 
 
-	std::function< v8::Local<v8::Value>(v8::Isolate *) > v8_call = [res](v8::Isolate *isolate) {
-		v8::Local<v8::Script> script = v8::Local<v8::Script>::New(isolate, *res->script);
-		return script->Run();
-	};
+	/* std::function relies on its dtor to be executed, otherwise it leaks
+	 * some memory on bailout. */
+	{
+		std::function< v8::Local<v8::Value>(v8::Isolate *) > v8_call = [res](v8::Isolate *isolate) {
+			v8::Local<v8::Script> script = v8::Local<v8::Script>::New(isolate, *res->script);
+			return script->Run();
+		};
+
+		v8js_v8_call(c, return_value, flags, time_limit, memory_limit, v8_call TSRMLS_CC);
+	}
 
 
-	v8js_v8_call(c, return_value, flags, time_limit, memory_limit, v8_call TSRMLS_CC);
+	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();
+	}
 }
 }
 
 
 /* {{{ proto mixed V8Js::executeString(string script [, string identifier [, int flags]])
 /* {{{ proto mixed V8Js::executeString(string script [, string identifier [, int flags]])

+ 92 - 93
v8js_v8.cc

@@ -76,133 +76,132 @@ void v8js_v8_init(TSRMLS_D) /* {{{ */
 /* }}} */
 /* }}} */
 
 
 
 
+/**
+ * Prepare V8 call trampoline with time & memory limit, exception handling, etc.
+ *
+ * The caller MUST check V8JSG(fatal_error_abort) and trigger further bailout
+ * either immediately after this function returns (or possibly after freeing
+ * heap allocated memory).
+ */
 void v8js_v8_call(v8js_ctx *c, zval **return_value,
 void v8js_v8_call(v8js_ctx *c, zval **return_value,
 				  long flags, long time_limit, long memory_limit,
 				  long flags, long time_limit, long memory_limit,
 				  std::function< v8::Local<v8::Value>(v8::Isolate *) >& v8_call TSRMLS_DC) /* {{{ */
 				  std::function< v8::Local<v8::Value>(v8::Isolate *) >& v8_call TSRMLS_DC) /* {{{ */
 {
 {
 	char *tz = NULL;
 	char *tz = NULL;
 
 
-	{
-		V8JS_CTX_PROLOGUE(c);
-
-		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;
+	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;
 
 
-		/* 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) {
-				v8::Date::DateTimeConfigurationChangeNotification(c->isolate);
+	/* 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) {
+			v8::Date::DateTimeConfigurationChangeNotification(c->isolate);
 
 
-		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 TSRMLS_CC);
+	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::Local<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 TSRMLS_CC);
 
 
-		/* 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::Local<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 TSRMLS_CC);
-				return;
-			}
+	if(!V8JSG(fatal_error_abort)) {
+		char exception_string[64];
 
 
-			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 TSRMLS_CC);
-				return;
-			}
+		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 TSRMLS_CC);
+			return;
+		}
 
 
-			if (!try_catch.CanContinue()) {
-				// At this point we can't re-throw the exception
-				return;
-			}
+		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 TSRMLS_CC);
+			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 TSRMLS_CC);
-				ZVAL_NULL(&c->pending_exception);
-			}
+		if (!try_catch.CanContinue()) {
+			// At this point we can't re-throw the exception
+			return;
+		}
 
 
-			/* Handle runtime JS exceptions */
-			if (try_catch.HasCaught()) {
+		/* 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 TSRMLS_CC);
+			ZVAL_NULL(&c->pending_exception);
+		}
 
 
-				/* Pending exceptions are set only in outer caller, inner caller exceptions are always rethrown */
-				if (c->in_execution < 1) {
+		/* Handle runtime JS exceptions */
+		if (try_catch.HasCaught()) {
 
 
-					/* Report immediately if report_uncaught is true */
-					if (c->report_uncaught) {
-						v8js_throw_script_exception(c->isolate, &try_catch TSRMLS_CC);
-						return;
-					}
+			/* Pending exceptions are set only in outer caller, inner caller exceptions are always rethrown */
+			if (c->in_execution < 1) {
 
 
-					/* Exception thrown from JS, preserve it for future execution */
-					if (result.IsEmpty()) {
-						v8js_create_script_exception(&c->pending_exception, c->isolate, &try_catch TSRMLS_CC);
-						return;
-					}
+				/* Report immediately if report_uncaught is true */
+				if (c->report_uncaught) {
+					v8js_throw_script_exception(c->isolate, &try_catch TSRMLS_CC);
+					return;
 				}
 				}
 
 
-				/* Rethrow back to JS */
-				try_catch.ReThrow();
-				return;
+				/* Exception thrown from JS, preserve it for future execution */
+				if (result.IsEmpty()) {
+					v8js_create_script_exception(&c->pending_exception, c->isolate, &try_catch TSRMLS_CC);
+					return;
+				}
 			}
 			}
 
 
-			/* Convert V8 value to PHP value */
-			if (!result.IsEmpty()) {
-				v8js_to_zval(result, *return_value, flags, c->isolate TSRMLS_CC);
-			}
+			/* Rethrow back to JS */
+			try_catch.ReThrow();
+			return;
 		}
 		}
-	} /* /V8JS_CTX_PROLOGUE */
 
 
-	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();
+		/* Convert V8 value to PHP value */
+		if (!result.IsEmpty()) {
+			v8js_to_zval(result, *return_value, flags, c->isolate TSRMLS_CC);
+		}
 	}
 	}
 }
 }
 /* }}} */
 /* }}} */

+ 40 - 30
v8js_v8object_class.cc

@@ -281,46 +281,56 @@ static int v8js_v8object_call_method(zend_string *method, zend_object *object, I
 		zend_get_parameters_array_ex(argc, argv);
 		zend_get_parameters_array_ex(argc, argv);
 	}
 	}
 
 
-	std::function< v8::Local<v8::Value>(v8::Isolate *) > v8_call = [obj, method, argc, argv TSRMLS_CC](v8::Isolate *isolate) {
-		int i = 0;
-
-		v8::Local<v8::String> method_name = V8JS_ZSYM(method);
-		v8::Local<v8::Object> v8obj = v8::Local<v8::Value>::New(isolate, obj->v8obj)->ToObject();
-		v8::Local<v8::Object> thisObj;
-		v8::Local<v8::Function> cb;
-
-		if (method_name->Equals(V8JS_SYM(V8JS_V8_INVOKE_FUNC_NAME))) {
-			cb = v8::Local<v8::Function>::Cast(v8obj);
-		} else {
-			cb = v8::Local<v8::Function>::Cast(v8obj->Get(method_name));
-		}
+	/* std::function relies on its dtor to be executed, otherwise it leaks
+	 * some memory on bailout. */
+	{
+		std::function< v8::Local<v8::Value>(v8::Isolate *) > v8_call = [obj, method, argc, argv TSRMLS_CC](v8::Isolate *isolate) {
+			int i = 0;
 
 
-		// 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::String> method_name = V8JS_ZSYM(method);
+			v8::Local<v8::Object> v8obj = v8::Local<v8::Value>::New(isolate, obj->v8obj)->ToObject();
+			v8::Local<v8::Object> thisObj;
+			v8::Local<v8::Function> cb;
 
 
-		v8::Local<v8::Value> *jsArgv = static_cast<v8::Local<v8::Value> *>(alloca(sizeof(v8::Local<v8::Value>) * argc));
-		v8::Local<v8::Value> js_retval;
+			if (method_name->Equals(V8JS_SYM(V8JS_V8_INVOKE_FUNC_NAME))) {
+				cb = v8::Local<v8::Function>::Cast(v8obj);
+			} else {
+				cb = v8::Local<v8::Function>::Cast(v8obj->Get(method_name));
+			}
 
 
-		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 TSRMLS_CC));
-		}
+			// 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));
+			v8::Local<v8::Value> js_retval;
 
 
-		return cb->Call(thisObj, argc, jsArgv);
-	};
+			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 TSRMLS_CC));
+			}
 
 
-	v8js_v8_call(obj->ctx, &return_value, obj->flags, obj->ctx->time_limit, obj->ctx->memory_limit, v8_call TSRMLS_CC);
+			return cb->Call(thisObj, argc, jsArgv);
+		};
+
+		v8js_v8_call(obj->ctx, &return_value, obj->flags, obj->ctx->time_limit, obj->ctx->memory_limit, v8_call TSRMLS_CC);
+	}
 
 
 	if (argc > 0) {
 	if (argc > 0) {
 		efree(argv);
 		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;
 	return SUCCESS;
 }
 }
 /* }}} */
 /* }}} */