diff --git a/php_v8js_macros.h b/php_v8js_macros.h index adfd5b1..59b4f12 100644 --- a/php_v8js_macros.h +++ b/php_v8js_macros.h @@ -196,11 +196,9 @@ struct php_v8js_object { ZEND_BEGIN_MODULE_GLOBALS(v8js) int v8_initialized; HashTable *extensions; - int disposed_contexts; /* Disposed contexts since last time V8 did GC */ /* Ini globals */ char *v8_flags; /* V8 command line flags */ - int max_disposed_contexts; /* Max disposed context allowed before forcing V8 GC */ // Timer thread globals std::stack timer_stack; diff --git a/tests/leak-php-object.phpt b/tests/leak-php-object.phpt new file mode 100644 index 0000000..20e7967 --- /dev/null +++ b/tests/leak-php-object.phpt @@ -0,0 +1,33 @@ +--TEST-- +Test V8::executeString() : Test for leaked PHP object if passed back multiple times +--SKIPIF-- + +--FILE-- +foo = new Foo(); +$v8->executeString($js); +unset($v8); + +?> +===EOF=== +--EXPECT-- +destroyed +===EOF=== diff --git a/v8js.cc b/v8js.cc index 8820304..f1d0621 100644 --- a/v8js.cc +++ b/v8js.cc @@ -39,20 +39,6 @@ ZEND_DECLARE_MODULE_GLOBALS(v8js) /* {{{ INI Settings */ -static ZEND_INI_MH(v8js_OnUpdateMaxDisposedContexts) /* {{{ */ -{ - int max_disposed = zend_atoi(new_value, new_value_length); - - if (max_disposed <= 0) { - return FAILURE; - } - - V8JSG(max_disposed_contexts) = max_disposed; - - return SUCCESS; -} -/* }}} */ - static ZEND_INI_MH(v8js_OnUpdateV8Flags) /* {{{ */ { if (new_value) { @@ -71,7 +57,6 @@ static ZEND_INI_MH(v8js_OnUpdateV8Flags) /* {{{ */ /* }}} */ ZEND_INI_BEGIN() /* {{{ */ - ZEND_INI_ENTRY("v8js.max_disposed_contexts", "25", ZEND_INI_ALL, v8js_OnUpdateMaxDisposedContexts) ZEND_INI_ENTRY("v8js.flags", NULL, ZEND_INI_ALL, v8js_OnUpdateV8Flags) ZEND_INI_END() /* }}} */ @@ -526,7 +511,18 @@ static void php_v8js_free_storage(void *object TSRMLS_DC) /* {{{ */ if (c->pending_exception) { zval_ptr_dtor(&c->pending_exception); } - + + /* Delete PHP global object from JavaScript */ + if (!c->context.IsEmpty()) { + v8::Locker locker(c->isolate); + v8::Isolate::Scope isolate_scope(c->isolate); + v8::HandleScope handle_scope(c->isolate); + v8::Context::Scope context_scope(c->isolate, c->context); + + v8::Local object_name_js = v8::Local::New(c->isolate, c->object_name); + V8JS_GLOBAL->Delete(object_name_js); + } + c->object_name.Reset(); c->object_name.~Persistent(); c->global_template.Reset(); @@ -542,14 +538,19 @@ static void php_v8js_free_storage(void *object TSRMLS_DC) /* {{{ */ /* Clear global object, dispose context */ if (!c->context.IsEmpty()) { c->context.Reset(); - V8JSG(disposed_contexts) = v8::V8::ContextDisposedNotification(); -#if V8JS_DEBUG - fprintf(stderr, "Context dispose notification sent (%d)\n", V8JSG(disposed_contexts)); - fflush(stderr); -#endif } c->context.~Persistent(); + /* Force garbage collection on our isolate, this is needed that V8 triggers + * our MakeWeak callbacks. Without these we won't remove our references + * on the PHP objects leading to memory leaks in PHP context. + */ + { + v8::Locker locker(c->isolate); + v8::Isolate::Scope isolate_scope(c->isolate); + while(!v8::V8::IdleNotification()) {}; + } + c->modules_stack.~vector(); c->modules_base.~vector(); efree(object); @@ -1621,28 +1622,6 @@ static PHP_MINIT_FUNCTION(v8js) } /* }}} */ -static void php_v8js_force_v8_gc(void) /* {{{ */ -{ -#if V8JS_DEBUG - TSRMLS_FETCH(); - fprintf(stderr, "############ Running V8 Idle notification: disposed/max_disposed %d/%d ############\n", V8JSG(disposed_contexts), V8JSG(max_disposed_contexts)); - fflush(stderr); -#endif - - while (!v8::V8::IdleNotification()) { -#if V8JS_DEBUG - fprintf(stderr, "."); - fflush(stderr); -#endif - } - -#if V8JS_DEBUG - fprintf(stderr, "\n"); - fflush(stderr); -#endif -} -/* }}} */ - /* {{{ PHP_MSHUTDOWN_FUNCTION */ static PHP_MSHUTDOWN_FUNCTION(v8js) @@ -1673,11 +1652,6 @@ static PHP_RSHUTDOWN_FUNCTION(v8js) fflush(stderr); #endif - /* Force V8 to do as much GC as possible */ - if (V8JSG(max_disposed_contexts) && (V8JSG(disposed_contexts) > V8JSG(max_disposed_contexts))) { - php_v8js_force_v8_gc(); - } - return SUCCESS; } /* }}} */ @@ -1702,8 +1676,6 @@ static PHP_MINFO_FUNCTION(v8js) static PHP_GINIT_FUNCTION(v8js) { v8js_globals->extensions = NULL; - v8js_globals->disposed_contexts = 0; - v8js_globals->max_disposed_contexts = 0; v8js_globals->v8_initialized = 0; v8js_globals->v8_flags = NULL; v8js_globals->timer_thread = NULL;