Browse Source

Merge pull request #151 from stesie/mem-leaks-001

Memory Leak Fixes
Stefan Siegl 9 years ago
parent
commit
204cc0fa6a
6 changed files with 50 additions and 21 deletions
  1. 1 0
      .gitignore
  2. 3 0
      php_v8js_macros.h
  3. 8 0
      v8js.cc
  4. 34 19
      v8js_class.cc
  5. 2 0
      v8js_class.h
  6. 2 2
      v8js_v8.cc

+ 1 - 0
.gitignore

@@ -34,3 +34,4 @@ v8js.la
 /tests/*.out
 /tests/*.php
 /tests/*.sh
+/tests/*.mem

+ 3 - 0
php_v8js_macros.h

@@ -115,6 +115,9 @@ void v8js_register_accessors(std::vector<v8js_accessor_ctx*> *accessor_list, v8:
 /* Module globals */
 ZEND_BEGIN_MODULE_GLOBALS(v8js)
   int v8_initialized;
+#if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
+  v8::Platform *v8_platform;
+#endif
   HashTable *extensions;
 
   /* Ini globals */

+ 8 - 0
v8js.cc

@@ -200,6 +200,14 @@ static PHP_GSHUTDOWN_FUNCTION(v8js)
 	v8js_globals->timer_stack.~deque();
 	v8js_globals->timer_mutex.~mutex();
 #endif
+
+	if (v8js_globals->v8_initialized) {
+		v8::V8::Dispose();
+		v8::V8::ShutdownPlatform();
+#if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
+		delete v8js_globals->v8_platform;
+#endif
+	}
 }
 /* }}} */
 

+ 34 - 19
v8js_class.cc

@@ -33,6 +33,7 @@ extern "C" {
 #include "v8js_timer.h"
 
 #include <functional>
+#include <algorithm>
 
 #define PHP_V8JS_SCRIPT_RES_NAME "V8Js script"
 
@@ -46,11 +47,11 @@ static zend_object_handlers v8js_object_handlers;
 
 typedef struct _v8js_script {
 	char *name;
-	v8::Isolate *isolate;	
+	v8js_ctx *ctx;
 	v8::Persistent<v8::Script, v8::CopyablePersistentTraits<v8::Script>> *script;
 } v8js_script;
 
-static void v8js_script_free(v8js_script *res, bool dispose_persistent);
+static void v8js_script_free(v8js_script *res);
 
 int le_v8js_script;
 
@@ -62,6 +63,7 @@ struct v8js_jsext {
 	int deps_count;
 	char *name;
 	char *source;
+	v8::Extension *extension;
 };
 /* }}} */
 
@@ -153,6 +155,13 @@ static void v8js_free_storage(void *object TSRMLS_DC) /* {{{ */
 	}
 	c->v8js_v8objects.~list();
 
+	for (std::vector<v8js_script *>::iterator it = c->script_objects.begin();
+		 it != c->script_objects.end(); it ++) {
+		(*it)->ctx = NULL;
+		(*it)->script->Reset();
+	}
+	c->script_objects.~vector();
+
 	/* Clear persistent handles in module cache */
 	for (std::map<char *, v8js_persistent_obj_t>::iterator it = c->modules_loaded.begin();
 		 it != c->modules_loaded.end(); ++it) {
@@ -209,6 +218,7 @@ static zend_object_value v8js_new(zend_class_entry *ce TSRMLS_DC) /* {{{ */
 	new(&c->weak_objects) std::map<zval *, v8js_persistent_obj_t>();
 
 	new(&c->v8js_v8objects) std::list<v8js_v8object *>();
+	new(&c->script_objects) std::vector<v8js_script *>();
 
 	retval.handle = zend_objects_store_put(c, NULL, (zend_objects_free_object_storage_t) v8js_free_storage, NULL TSRMLS_CC);
 	retval.handlers = &v8js_object_handlers;
@@ -241,6 +251,7 @@ static void v8js_jsext_dtor(v8js_jsext *jsext) /* {{{ */
 	if (jsext->deps) {
 		v8js_free_ext_strarr(jsext->deps, jsext->deps_count);
 	}
+	delete jsext->extension;
 	free(jsext->name);
 	free(jsext->source);
 }
@@ -494,7 +505,7 @@ static void v8js_compile_script(zval *this_ptr, const char *str, int str_len, co
 
 	v8::String::Utf8Value _sname(sname);
 	res->name = estrndup(ToCString(_sname), _sname.length());
-	res->isolate = c->isolate;
+	res->ctx = c;
 	*ret = res;
 	return;
 }
@@ -503,7 +514,7 @@ static void v8js_execute_script(zval *this_ptr, v8js_script *res, long flags, lo
 {
 	v8js_ctx *c = (v8js_ctx *) zend_object_store_get_object(getThis() TSRMLS_CC);
 
-	if (res->isolate != c->isolate) {
+	if (res->ctx != c) {
 		zend_error(E_WARNING, "Script resource from wrong V8Js object passed");
 		ZVAL_BOOL(*return_value, 0);
 		return;
@@ -543,7 +554,7 @@ static PHP_METHOD(V8Js, executeString)
 		RETURN_FALSE;
 	}
 	v8js_execute_script(getThis(), res, flags, time_limit, memory_limit, &return_value TSRMLS_CC);
-	v8js_script_free(res, true);
+	v8js_script_free(res);
 	efree(res);
 }
 /* }}} */
@@ -564,6 +575,10 @@ static PHP_METHOD(V8Js, compileString)
 	v8js_compile_script(getThis(), str, str_len, identifier, identifier_len, &res TSRMLS_CC);
 	if (res) {
 		ZEND_REGISTER_RESOURCE(return_value, res, le_v8js_script);
+
+		v8js_ctx *ctx;
+		ctx = (v8js_ctx *) zend_object_store_get_object(this_ptr TSRMLS_CC);
+		ctx->script_objects.push_back(res);
 	}
 	return;
 }
@@ -609,7 +624,7 @@ static PHP_METHOD(V8Js, checkString)
 		RETURN_FALSE;
 	}
 
-	v8js_script_free(res, true);
+	v8js_script_free(res);
 	efree(res);
 	RETURN_TRUE;
 }
@@ -766,23 +781,22 @@ static void v8js_persistent_zval_dtor(zval **p) /* {{{ */
 }
 /* }}} */
 
-static void v8js_script_free(v8js_script *res, bool dispose_persistent)
+static void v8js_script_free(v8js_script *res)
 {
-	if (res->name) {
-		efree(res->name);
-		res->name = NULL;
-	}
-	if (dispose_persistent) {
-		res->script->~Persistent(); // does Reset()
-		res->script = NULL;
-	}
+	efree(res->name);
+	delete res->script; // does Reset()
 }
 
 static void v8js_script_dtor(zend_rsrc_list_entry *rsrc TSRMLS_DC) /* {{{ */
 {
 	v8js_script *res = (v8js_script *)rsrc->ptr;
 	if (res) {
-		v8js_script_free(res, false);
+		if(res->ctx) {
+			std::vector<v8js_script *>::iterator it = std::find(res->ctx->script_objects.begin(), res->ctx->script_objects.end(), res);
+			res->ctx->script_objects.erase(it);
+		}
+
+		v8js_script_free(res);
 		efree(res);
 	}
 }
@@ -822,15 +836,16 @@ static int v8js_register_extension(char *name, uint name_len, char *source, uint
 		zend_hash_copy(jsext->deps_ht, Z_ARRVAL_P(deps_arr), (copy_ctor_func_t) v8js_persistent_zval_ctor, NULL, sizeof(zval *));
 	}
 
+	jsext->extension = new v8::Extension(jsext->name, jsext->source, jsext->deps_count, jsext->deps);
+
 	if (zend_hash_add(V8JSG(extensions), name, name_len + 1, jsext, sizeof(v8js_jsext), NULL) == FAILURE) {
 		v8js_jsext_dtor(jsext);
 		free(jsext);
 		return FAILURE;
 	}
 
-	v8::Extension *extension = new v8::Extension(jsext->name, jsext->source, jsext->deps_count, jsext->deps);
-	extension->set_auto_enable(auto_enable ? true : false);
-	v8::RegisterExtension(extension);
+	jsext->extension->set_auto_enable(auto_enable ? true : false);
+	v8::RegisterExtension(jsext->extension);
 
 	free(jsext);
 

+ 2 - 0
v8js_class.h

@@ -22,6 +22,7 @@ typedef v8::Persistent<v8::Object, v8::CopyablePersistentTraits<v8::Object> > v8
 /* Forward declarations */
 struct v8js_v8object;
 struct v8js_accessor_ctx;
+struct _v8js_script;
 
 /* {{{ Context container */
 struct v8js_ctx {
@@ -52,6 +53,7 @@ struct v8js_ctx {
   std::list<v8js_v8object *> v8js_v8objects;
 
   std::vector<v8js_accessor_ctx *> accessor_list;
+  std::vector<struct _v8js_script *> script_objects;
   char *tz;
 #ifdef ZTS
   void ***zts_ctx;

+ 2 - 2
v8js_v8.cc

@@ -42,8 +42,8 @@ void v8js_v8_init(TSRMLS_D) /* {{{ */
 	}
 
 #if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
-	v8::Platform* platform = v8::platform::CreateDefaultPlatform();
-	v8::V8::InitializePlatform(platform);
+	V8JSG(v8_platform) = v8::platform::CreateDefaultPlatform();
+	v8::V8::InitializePlatform(V8JSG(v8_platform));
 #endif
 
 	/* Set V8 command line flags (must be done before V8::Initialize()!) */