mirror of
https://github.com/phpv8/v8js.git
synced 2024-11-09 16:28:41 +00:00
Trigger garbage collection within Isolate::Scope
Before the idle notifications to V8 were sent without a special Isolate entered, which results in V8 using it's default isolate (which we don't use at all). Hence those were pretty useless anyways (if they were called, which was unlikely). Besides V8 seems to not trigger the weak object callbacks if the isolate is destroyed, hence the PHP objects we add-ref'ed before will leak. Therefore this removes the previous idle notification logic and forces garbage collection from the V8Js object destructor.
This commit is contained in:
parent
5c1c68b045
commit
18b129b128
@ -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<php_v8js_timer_ctx *> timer_stack;
|
||||
|
33
tests/leak-php-object.phpt
Normal file
33
tests/leak-php-object.phpt
Normal file
@ -0,0 +1,33 @@
|
||||
--TEST--
|
||||
Test V8::executeString() : Test for leaked PHP object if passed back multiple times
|
||||
--SKIPIF--
|
||||
<?php require_once(dirname(__FILE__) . '/skipif.inc'); ?>
|
||||
--FILE--
|
||||
<?php
|
||||
|
||||
$js =<<< EOF
|
||||
for(var i = 0; i < 1000; i ++) {
|
||||
PHP.foo.getStdClassObject();
|
||||
}
|
||||
EOF;
|
||||
|
||||
class Foo {
|
||||
public function getStdClassObject() {
|
||||
return new stdClass();
|
||||
}
|
||||
|
||||
public function __destruct() {
|
||||
echo "destroyed\n";
|
||||
}
|
||||
}
|
||||
|
||||
$v8 = new V8Js();
|
||||
$v8->foo = new Foo();
|
||||
$v8->executeString($js);
|
||||
unset($v8);
|
||||
|
||||
?>
|
||||
===EOF===
|
||||
--EXPECT--
|
||||
destroyed
|
||||
===EOF===
|
59
v8js.cc
59
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()
|
||||
/* }}} */
|
||||
@ -542,14 +527,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 +1611,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 +1641,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 +1665,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;
|
||||
|
Loading…
Reference in New Issue
Block a user