From a83c49266e6896c0d7ccdbe480e8aafb61040a47 Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Fri, 1 Jan 2016 19:10:04 +0100 Subject: [PATCH] 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. --- v8js_class.cc | 20 +++- v8js_v8.cc | 201 ++++++++++++++++++++--------------------- v8js_v8object_class.cc | 66 ++++++++------ 3 files changed, 153 insertions(+), 134 deletions(-) diff --git a/v8js_class.cc b/v8js_class.cc index 1548d4b..bc84f7a 100644 --- a/v8js_class.cc +++ b/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; } - std::function< v8::Local(v8::Isolate *) > v8_call = [res](v8::Isolate *isolate) { - v8::Local script = v8::Local::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::Isolate *) > v8_call = [res](v8::Isolate *isolate) { + v8::Local script = v8::Local::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]]) diff --git a/v8js_v8.cc b/v8js_v8.cc index 2bad10a..ea4deae 100644 --- a/v8js_v8.cc +++ b/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, long flags, long time_limit, long memory_limit, std::function< v8::Local(v8::Isolate *) >& v8_call TSRMLS_DC) /* {{{ */ { char *tz = NULL; - { - V8JS_CTX_PROLOGUE(c); + V8JS_CTX_PROLOGUE(c); - V8JSG(timer_mutex).lock(); - c->time_limit_hit = false; - c->memory_limit_hit = false; - V8JSG(timer_mutex).unlock(); + V8JSG(timer_mutex).lock(); + c->time_limit_hit = false; + c->memory_limit_hit = false; + V8JSG(timer_mutex).unlock(); - /* Catch JS exceptions */ - v8::TryCatch try_catch; + /* Catch JS exceptions */ + v8::TryCatch try_catch; - /* Set flags for runtime use */ - c->flags = flags; + /* Set flags for runtime use */ + c->flags = flags; - /* Check if timezone has been changed and notify V8 */ - tz = getenv("TZ"); + /* Check if timezone has been changed and notify V8 */ + tz = getenv("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 (tz != NULL) { + if (c->tz == NULL) { + c->tz = strdup(tz); + } + else if (strcmp(c->tz, tz) != 0) { + v8::Date::DateTimeConfigurationChangeNotification(c->isolate); - free(c->tz); - c->tz = strdup(tz); - } + free(c->tz); + c->tz = strdup(tz); + } + } + + 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)); + } + } + + /* 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); + + /* Execute script */ + c->in_execution++; + v8::Local result = v8_call(c->isolate); + c->in_execution--; + + /* 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(); + + time_limit = timer_ctx->time_limit; + memory_limit = timer_ctx->memory_limit; + + efree(timer_ctx); + + if(!V8JSG(fatal_error_abort)) { + char exception_string[64]; + + 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 (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)); - } + 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; } - /* 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 (!try_catch.CanContinue()) { + // At this point we can't re-throw the exception + return; + } - /* Execute script */ - c->in_execution++; - v8::Local result = v8_call(c->isolate); - c->in_execution--; + /* 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); + } - /* 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(); + /* Handle runtime JS exceptions */ + if (try_catch.HasCaught()) { - time_limit = timer_ctx->time_limit; - memory_limit = timer_ctx->memory_limit; + /* Pending exceptions are set only in outer caller, inner caller exceptions are always rethrown */ + if (c->in_execution < 1) { - efree(timer_ctx); - - if(!V8JSG(fatal_error_abort)) { - char exception_string[64]; - - 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 (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 (!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 TSRMLS_CC); - ZVAL_NULL(&c->pending_exception); - } - - /* 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 TSRMLS_CC); - 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; - } + /* 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); + } } } /* }}} */ diff --git a/v8js_v8object_class.cc b/v8js_v8object_class.cc index a4652f8..3e1f6ea 100644 --- a/v8js_v8object_class.cc +++ b/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); } - std::function< v8::Local(v8::Isolate *) > v8_call = [obj, method, argc, argv TSRMLS_CC](v8::Isolate *isolate) { - int i = 0; + /* std::function relies on its dtor to be executed, otherwise it leaks + * some memory on bailout. */ + { + std::function< v8::Local(v8::Isolate *) > v8_call = [obj, method, argc, argv TSRMLS_CC](v8::Isolate *isolate) { + int i = 0; - v8::Local method_name = V8JS_ZSYM(method); - v8::Local v8obj = v8::Local::New(isolate, obj->v8obj)->ToObject(); - v8::Local thisObj; - v8::Local cb; + v8::Local method_name = V8JS_ZSYM(method); + v8::Local v8obj = v8::Local::New(isolate, obj->v8obj)->ToObject(); + v8::Local thisObj; + v8::Local cb; - if (method_name->Equals(V8JS_SYM(V8JS_V8_INVOKE_FUNC_NAME))) { - cb = v8::Local::Cast(v8obj); - } else { - cb = v8::Local::Cast(v8obj->Get(method_name)); - } + if (method_name->Equals(V8JS_SYM(V8JS_V8_INVOKE_FUNC_NAME))) { + cb = v8::Local::Cast(v8obj); + } else { + cb = v8::Local::Cast(v8obj->Get(method_name)); + } - // 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); - } + // 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 *jsArgv = static_cast *>(alloca(sizeof(v8::Local) * argc)); - v8::Local js_retval; + v8::Local *jsArgv = static_cast *>(alloca(sizeof(v8::Local) * argc)); + v8::Local js_retval; - for (i = 0; i < argc; i++) { - new(&jsArgv[i]) v8::Local; - jsArgv[i] = v8::Local::New(isolate, zval_to_v8js(&argv[i], isolate TSRMLS_CC)); - } + for (i = 0; i < argc; i++) { + new(&jsArgv[i]) v8::Local; + jsArgv[i] = v8::Local::New(isolate, zval_to_v8js(&argv[i], isolate TSRMLS_CC)); + } - return cb->Call(thisObj, argc, jsArgv); - }; + 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); + v8js_v8_call(obj->ctx, &return_value, obj->flags, obj->ctx->time_limit, obj->ctx->memory_limit, v8_call TSRMLS_CC); + } 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; } /* }}} */