From 57348c5f7d7bca084b6ef64724b78eaea6a08c78 Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Sat, 26 Oct 2013 22:57:35 -0400 Subject: [PATCH] Fix double-pop of timer_ctx (and efree from wrong thread). The timer_ctx was being popped & freed in terminate_execution, from the timer thread (not the main thread), and then popped again in the main thread when execution was aborted. Do all pop/free work in main thread. Also, remember to pop the timer_ctx when just a memory limit is set. --- php_v8js_macros.h | 1 + v8js.cc | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/php_v8js_macros.h b/php_v8js_macros.h index da5cc5e..38e35e1 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -180,6 +180,7 @@ struct php_v8js_timer_ctx long memory_limit; std::chrono::time_point time_point; php_v8js_ctx *v8js_ctx; + bool killed; }; /* {{{ Object container */ diff --git a/v8js.cc b/v8js.cc index bcc8174..d90e926 100644 --- a/v8js.cc +++ b/v8js.cc @@ -889,6 +889,7 @@ static void php_v8js_timer_push(long time_limit, long memory_limit, php_v8js_ctx timer_ctx->memory_limit = memory_limit; timer_ctx->time_point = from + duration; timer_ctx->v8js_ctx = c; + timer_ctx->killed = false; V8JSG(timer_stack).push(timer_ctx); V8JSG(timer_mutex).unlock(); @@ -899,12 +900,11 @@ static void php_v8js_timer_pop(TSRMLS_D) V8JSG(timer_mutex).lock(); if (V8JSG(timer_stack).size()) { - // Free the timer context memory php_v8js_timer_ctx *timer_ctx = V8JSG(timer_stack).top(); - efree(timer_ctx); - // Remove the timer context from the stack V8JSG(timer_stack).pop(); + // Free the timer context memory + efree(timer_ctx); } V8JSG(timer_mutex).unlock(); @@ -914,9 +914,7 @@ static void php_v8js_terminate_execution(php_v8js_ctx *c TSRMLS_DC) { // Forcefully terminate the current thread of V8 execution in the isolate v8::V8::TerminateExecution(c->isolate); - - // Remove this timer from the stack - php_v8js_timer_pop(TSRMLS_C); + // This timer will be removed from stack by the parent thread. } static void php_v8js_timer_thread(TSRMLS_D) @@ -933,7 +931,9 @@ static void php_v8js_timer_thread(TSRMLS_D) // Get memory usage statistics for the isolate c->isolate->GetHeapStatistics(&hs); - if (timer_ctx->time_limit > 0 && now > timer_ctx->time_point) { + if (timer_ctx->time_limit > 0 && now > timer_ctx->time_point && + !timer_ctx->killed) { + timer_ctx->killed = true; php_v8js_terminate_execution(c TSRMLS_CC); V8JSG(timer_mutex).lock(); @@ -941,7 +941,9 @@ static void php_v8js_timer_thread(TSRMLS_D) V8JSG(timer_mutex).unlock(); } - if (timer_ctx->memory_limit > 0 && hs.used_heap_size() > timer_ctx->memory_limit) { + if (timer_ctx->memory_limit > 0 && hs.used_heap_size() > timer_ctx->memory_limit && + !timer_ctx->killed) { + timer_ctx->killed = true; php_v8js_terminate_execution(c TSRMLS_CC); V8JSG(timer_mutex).lock(); @@ -1020,7 +1022,7 @@ static PHP_METHOD(V8Js, executeString) v8::Local result = script->Run(); c->in_execution--; - if (time_limit > 0) { + if (time_limit > 0 || memory_limit > 0) { php_v8js_timer_pop(TSRMLS_C); }