Solve bugs in destruction of node loader, some leaks are still present. Add debug info for functions, classes and objects in order to detect leaks.

viferga · viferga · commit 715a2f5948f1 · 2021-09-10T02:38:54.000+02:00
diff --git a/source/loader/include/loader/loader.h b/source/loader/include/loader/loader.h
@@ -93,7 +93,7 @@ LOADER_API value loader_metadata(void);
 
 LOADER_API int loader_clear(void *handle);
 
-LOADER_API void loader_unload_children(loader_impl impl);
+LOADER_API void loader_unload_children(loader_impl impl, int destroy_objects);
 
 LOADER_API int loader_unload(void);
 
diff --git a/source/loader/source/loader.c b/source/loader/source/loader.c
@@ -760,7 +760,7 @@ int loader_clear(void *handle)
 	return loader_impl_clear(handle);
 }
 
-void loader_unload_children(loader_impl impl)
+void loader_unload_children(loader_impl impl, int destroy_objects)
 {
 	loader l = loader_singleton();
 	uint64_t current = thread_id_get_current();
@@ -806,7 +806,7 @@ void loader_unload_children(loader_impl impl)
 	vector_destroy(stack);
 
 	/* Clear all objects and types related to the loader once all childs have been destroyed */
-	if (impl != NULL)
+	if (impl != NULL && destroy_objects == 0)
 	{
 		loader_impl_destroy_objects(impl);
 	}
@@ -833,7 +833,7 @@ int loader_unload()
 			/* TODO: How to deal with this? */
 		}
 
-		loader_unload_children(NULL);
+		loader_unload_children(NULL, 1);
 
 		/* The proxy is the first loader, it must be destroyed at the end */
 		if (l->proxy != NULL)
diff --git a/source/loaders/c_loader/source/c_loader_impl.cpp b/source/loaders/c_loader/source/c_loader_impl.cpp
@@ -289,7 +289,7 @@ int c_loader_impl_destroy(loader_impl impl)
 	if (c_impl != NULL)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		delete c_impl;
 
diff --git a/source/loaders/cob_loader/source/cob_loader_impl.cpp b/source/loaders/cob_loader/source/cob_loader_impl.cpp
@@ -263,7 +263,7 @@ int cob_loader_impl_destroy(loader_impl impl)
 	(void)impl;
 
 	// Destroy children loaders
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	return cobtidy();
 }
diff --git a/source/loaders/cr_loader/crystal/cr_loader_impl.cr b/source/loaders/cr_loader/crystal/cr_loader_impl.cr
@@ -78,7 +78,7 @@ fun cr_loader_impl_destroy(impl : Void*) : LibC::Int
 	ptr = LibMetaCall.loader_impl_get(impl)
 
 	# Destroy children loaded by this loader
-	LibMetaCall.loader_unload_children()
+	LibMetaCall.loader_unload_children(impl, 0)
 
 	cr_impl = Box(CrystalLoaderImpl).unbox(ptr)
 
diff --git a/source/loaders/cr_loader/crystal/metacall.cr b/source/loaders/cr_loader/crystal/metacall.cr
@@ -21,7 +21,7 @@ lib LibMetaCall
 	# Loader
 	fun loader_impl_get(impl : Void*) : Void*
 	fun loader_initialization_register(impl : Void*) : Void
-	fun loader_unload_children() : Void
+	fun loader_unload_children(impl : Void*, destroy_children : int) : Void
 
 	# Log
 	# TODO: log_write is a macro that must be reimplemented properly (log_write_impl, log_write_impl_va)
diff --git a/source/loaders/cs_loader/source/cs_loader_impl.c b/source/loaders/cs_loader/source/cs_loader_impl.c
@@ -396,7 +396,7 @@ int cs_loader_impl_destroy(loader_impl impl)
 	netcore_handle nhandle = (netcore_handle)loader_impl_get(impl);
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	simple_netcore_destroy(nhandle);
 
diff --git a/source/loaders/dart_loader/source/dart_loader_impl.cc b/source/loaders/dart_loader/source/dart_loader_impl.cc
@@ -571,7 +571,7 @@ int dart_loader_impl_destroy(loader_impl impl)
 	if (dart_impl != nullptr)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		delete dart_impl;
 
diff --git a/source/loaders/file_loader/source/file_loader_impl.c b/source/loaders/file_loader/source/file_loader_impl.c
@@ -493,7 +493,7 @@ int file_loader_impl_destroy(loader_impl impl)
 	if (file_impl != NULL)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		if (file_impl->execution_paths != NULL)
 		{
diff --git a/source/loaders/java_loader/source/java_loader_impl.cpp b/source/loaders/java_loader/source/java_loader_impl.cpp
@@ -2023,7 +2023,7 @@ int java_loader_impl_destroy(loader_impl impl)
 		}
 
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		java_impl->jvm->DestroyJavaVM();
 
diff --git a/source/loaders/jl_loader/source/jl_loader_impl.cpp b/source/loaders/jl_loader/source/jl_loader_impl.cpp
@@ -337,7 +337,7 @@ int jl_loader_impl_destroy(loader_impl impl)
 	loader_impl_jl jl_impl = static_cast<loader_impl_jl>(loader_impl_get(impl));
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	/* TODO */
 	// Clear here the resources, it must free all memory related to jl_impl.
diff --git a/source/loaders/js_loader/source/js_loader_impl.cpp b/source/loaders/js_loader/source/js_loader_impl.cpp
@@ -848,7 +848,7 @@ int js_loader_impl_destroy(loader_impl impl)
 	if (js_impl != nullptr)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		/* Destroy V8 */
 		if (js_impl->isolate_scope != nullptr)
diff --git a/source/loaders/jsm_loader/source/jsm_loader_impl.cpp b/source/loaders/jsm_loader/source/jsm_loader_impl.cpp
@@ -235,7 +235,7 @@ int jsm_loader_impl_destroy(loader_impl impl)
 	if (jsm_impl != NULL)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		/* Destroy Spider Monkey */
 		if (jsm_impl->cx != NULL)
diff --git a/source/loaders/llvm_loader/source/llvm_loader_impl.cpp b/source/loaders/llvm_loader/source/llvm_loader_impl.cpp
@@ -369,7 +369,7 @@ int llvm_loader_impl_destroy(loader_impl impl)
 	loader_impl_llvm llvm_impl = static_cast<loader_impl_llvm>(loader_impl_get(impl));
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	/* TODO */
 	// Clear here the resources, it must free all memory related to llvm_impl.
diff --git a/source/loaders/lua_loader/source/lua_loader_impl.c b/source/loaders/lua_loader/source/lua_loader_impl.c
@@ -507,7 +507,7 @@ int lua_loader_impl_destroy(loader_impl impl)
 	if (lua_impl != NULL)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		/* Destroy Lua VM */
 		lua_close(lua_impl->vm);
diff --git a/source/loaders/mock_loader/source/mock_loader_impl.c b/source/loaders/mock_loader/source/mock_loader_impl.c
@@ -571,7 +571,7 @@ int mock_loader_impl_destroy(loader_impl impl)
 	if (mock_impl != NULL)
 	{
 		/* Destroy children loaders */
-		loader_unload_children(impl);
+		loader_unload_children(impl, 0);
 
 		/* Destroy Mock */
 		free(mock_impl);
diff --git a/source/loaders/node_loader/source/node_loader_impl.cpp b/source/loaders/node_loader/source/node_loader_impl.cpp
@@ -1122,7 +1122,11 @@ napi_value node_loader_impl_value_to_napi(loader_impl_node node_impl, napi_env e
 	}
 	else
 	{
-		napi_throw_error(env, NULL, "NodeJS Loader could not convert the value to N-API");
+		std::string error_str("NodeJS Loader could not convert the value of type '");
+		error_str += type_id_name(id);
+		error_str += "' to N-API";
+
+		napi_throw_error(env, NULL, error_str.c_str());
 	}
 
 	return v;
@@ -3989,6 +3993,9 @@ void node_loader_impl_thread(void *data)
 	/* Start NodeJS runtime */
 	int result = node::Start(argc, reinterpret_cast<char **>(argv));
 
+	/* Destroy children loaders */
+	loader_unload_children(node_impl->impl, 1);
+
 	/* Lock node implementation mutex */
 	uv_mutex_lock(&node_impl->mutex);
 
@@ -4727,16 +4734,17 @@ int64_t node_loader_impl_async_handles_count(loader_impl_node node_impl)
 
 int64_t node_loader_impl_user_async_handles_count(loader_impl_node node_impl)
 {
+	int64_t active_handles = node_loader_impl_async_handles_count(node_impl);
+
+#if (!defined(NDEBUG) || defined(DEBUG) || defined(_DEBUG) || defined(__DEBUG) || defined(__DEBUG__))
 	int64_t closing =
-#if defined(WIN32) || defined(_WIN32)
+	#if defined(WIN32) || defined(_WIN32)
 		(node_impl->thread_loop->endgame_handles != NULL)
-#else
+	#else
 		(node_impl->thread_loop->closing_handles != NULL)
-#endif
+	#endif
 		;
 
-	int64_t active_handles = node_loader_impl_async_handles_count(node_impl);
-#if (!defined(NDEBUG) || defined(DEBUG) || defined(_DEBUG) || defined(__DEBUG) || defined(__DEBUG__))
 	printf("[active_handles] - [base_active_handles] - [extra_active_handles] + [active_reqs] + [closing]\n");
 	printf("       %" PRId64 "        -           %" PRId64 "          -            %" PRId64 "           +       %" PRId64 "       +     %" PRId64 "\n",
 		active_handles,
@@ -4745,7 +4753,8 @@ int64_t node_loader_impl_user_async_handles_count(loader_impl_node node_impl)
 		(int64_t)node_impl->thread_loop->active_reqs.count,
 		closing);
 #endif
-	return active_handles - node_impl->base_active_handles - node_impl->extra_active_handles.load() + (int64_t)(node_impl->thread_loop->active_reqs.count) + closing;
+
+	return active_handles - node_impl->base_active_handles - node_impl->extra_active_handles.load() + (int64_t)(node_impl->thread_loop->active_reqs.count) /*+ closing*/;
 }
 
 void node_loader_impl_print_handles(loader_impl_node node_impl)
@@ -4811,8 +4820,8 @@ void node_loader_impl_destroy_safe_impl(loader_impl_node node_impl, napi_env env
 	uint32_t ref_count = 0;
 	napi_status status;
 
-	/* Destroy children loaders (any loader initialized in the current V8 thread should be deleted first) */
-	loader_unload_children(node_impl->impl);
+	/* Clear all the functions and classes loaded from node */
+	loader_impl_destroy_objects(node_impl->impl);
 
 	/* Clear thread safe functions */
 	{
diff --git a/source/loaders/node_loader/source/node_loader_port.cpp b/source/loaders/node_loader/source/node_loader_port.cpp
@@ -276,7 +276,11 @@ napi_value node_loader_port_load_from_file_export(napi_env env, napi_callback_in
 
 	void *exports = metacall_handle_export(handle);
 
-	return node_loader_impl_value_to_napi(node_impl, env, exports);
+	napi_value v_exports = node_loader_impl_value_to_napi(node_impl, env, exports);
+
+	node_loader_impl_finalizer(env, v_exports, exports);
+
+	return v_exports;
 }
 
 /**
diff --git a/source/loaders/py_loader/source/py_loader_impl.c b/source/loaders/py_loader/source/py_loader_impl.c
@@ -1604,7 +1604,9 @@ void function_py_interface_destroy(function func, function_impl impl)
 			free(py_func->values);
 		}
 
+		PyGILState_STATE gstate = PyGILState_Ensure();
 		Py_DECREF(py_func->func);
+		PyGILState_Release(gstate);
 
 		free(py_func);
 	}
@@ -3745,7 +3747,7 @@ int py_loader_impl_destroy(loader_impl impl)
 	}
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	Py_DECREF(py_impl->inspect_signature);
 	Py_DECREF(py_impl->inspect_getattr_static);
diff --git a/source/loaders/rb_loader/source/rb_loader_impl.c b/source/loaders/rb_loader/source/rb_loader_impl.c
@@ -1535,7 +1535,7 @@ int rb_loader_impl_destroy(loader_impl impl)
 	(void)impl;
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	return ruby_cleanup(0);
 }
diff --git a/source/loaders/rpc_loader/source/rpc_loader_impl.cpp b/source/loaders/rpc_loader/source/rpc_loader_impl.cpp
@@ -674,7 +674,7 @@ int rpc_loader_impl_destroy(loader_impl impl)
 	loader_impl_rpc rpc_impl = static_cast<loader_impl_rpc>(loader_impl_get(impl));
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	metacall_allocator_destroy(rpc_impl->allocator);
 
diff --git a/source/loaders/ts_loader/source/ts_loader_impl.cpp b/source/loaders/ts_loader/source/ts_loader_impl.cpp
@@ -364,7 +364,7 @@ int ts_loader_impl_destroy(loader_impl impl)
 	}
 
 	/* Destroy children loaders */
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 
 	return metacall_clear(ts_impl);
 }
diff --git a/source/loaders/wasm_loader/source/wasm_loader_impl.c b/source/loaders/wasm_loader/source/wasm_loader_impl.c
@@ -231,7 +231,7 @@ int wasm_loader_impl_discover(loader_impl impl, loader_handle handle, context ct
 int wasm_loader_impl_destroy(loader_impl impl)
 {
 	loader_impl_wasm wasm_impl = loader_impl_get(impl);
-	loader_unload_children(impl);
+	loader_unload_children(impl, 0);
 	vector_destroy(wasm_impl->paths);
 	wasm_store_delete(wasm_impl->store);
 	wasm_engine_delete(wasm_impl->engine);
diff --git a/source/metacall/source/metacall.c b/source/metacall/source/metacall.c
@@ -2024,6 +2024,12 @@ int metacall_destroy()
 		configuration_destroy();
 
 		metacall_initialize_flag = 1;
+
+#if (!defined(NDEBUG) || defined(DEBUG) || defined(_DEBUG) || defined(__DEBUG) || defined(__DEBUG__))
+		function_stats_debug();
+		class_stats_debug();
+		object_stats_debug();
+#endif
 	}
 
 	return 0;
diff --git a/source/reflect/include/reflect/reflect.h b/source/reflect/include/reflect/reflect.h
@@ -23,9 +23,11 @@
 
 #include <reflect/reflect_api.h>
 
+#include <reflect/reflect_class.h>
 #include <reflect/reflect_context.h>
 #include <reflect/reflect_function.h>
 #include <reflect/reflect_future.h>
+#include <reflect/reflect_object.h>
 #include <reflect/reflect_scope.h>
 #include <reflect/reflect_signature.h>
 #include <reflect/reflect_type.h>
diff --git a/source/reflect/include/reflect/reflect_class.h b/source/reflect/include/reflect/reflect_class.h
@@ -120,6 +120,8 @@ REFLECT_API const char *class_name(klass cls);
 
 REFLECT_API value class_metadata(klass cls);
 
+REFLECT_API void class_stats_debug(void);
+
 REFLECT_API void class_destroy(klass cls);
 
 #ifdef __cplusplus
diff --git a/source/reflect/include/reflect/reflect_function.h b/source/reflect/include/reflect/reflect_function.h
@@ -86,6 +86,8 @@ REFLECT_API function_return function_call(function func, function_args args, siz
 
 REFLECT_API function_return function_await(function func, function_args args, size_t size, function_resolve_callback resolve_callback, function_reject_callback reject_callback, void *context);
 
+REFLECT_API void function_stats_debug(void);
+
 REFLECT_API void function_destroy(function func);
 
 #ifdef __cplusplus
diff --git a/source/reflect/include/reflect/reflect_object.h b/source/reflect/include/reflect/reflect_object.h
@@ -98,6 +98,8 @@ REFLECT_API value object_metadata(object obj);
 
 REFLECT_API int object_delete(object obj);
 
+REFLECT_API void object_stats_debug(void);
+
 REFLECT_API void object_destroy(object obj);
 
 #ifdef __cplusplus
diff --git a/source/reflect/source/reflect_class.c b/source/reflect/source/reflect_class.c
diff --git a/source/reflect/source/reflect_function.c b/source/reflect/source/reflect_function.c
diff --git a/source/reflect/source/reflect_object.c b/source/reflect/source/reflect_object.c

Original file line number	Diff line number	Diff line change
`@@ -760,7 +760,7 @@ int loader_clear(void *handle)`
`760`	`760`	`return loader_impl_clear(handle);`
`761`	`761`	`}`
`762`	`762`
`763`		`-void loader_unload_children(loader_impl impl)`
	`763`	`+void loader_unload_children(loader_impl impl, int destroy_objects)`
`764`	`764`	`{`
`765`	`765`	`loader l = loader_singleton();`
`766`	`766`	`uint64_t current = thread_id_get_current();`
`@@ -806,7 +806,7 @@ void loader_unload_children(loader_impl impl)`
`806`	`806`	`vector_destroy(stack);`
`807`	`807`
`808`	`808`	`/* Clear all objects and types related to the loader once all childs have been destroyed */`
`809`		`- if (impl != NULL)`
	`809`	`+ if (impl != NULL && destroy_objects == 0)`
`810`	`810`	`{`
`811`	`811`	`loader_impl_destroy_objects(impl);`
`812`	`812`	`}`
`@@ -833,7 +833,7 @@ int loader_unload()`
`833`	`833`	`/* TODO: How to deal with this? */`
`834`	`834`	`}`
`835`	`835`
`836`		`- loader_unload_children(NULL);`
	`836`	`+ loader_unload_children(NULL, 1);`
`837`	`837`
`838`	`838`	`/* The proxy is the first loader, it must be destroyed at the end */`
`839`	`839`	`if (l->proxy != NULL)`
Original file line number	Diff line number	Diff line change
`@@ -289,7 +289,7 @@ int c_loader_impl_destroy(loader_impl impl)`
`289`	`289`	`if (c_impl != NULL)`
`290`	`290`	`{`
`291`	`291`	`/* Destroy children loaders */`
`292`		`- loader_unload_children(impl);`
	`292`	`+ loader_unload_children(impl, 0);`
`293`	`293`
`294`	`294`	`delete c_impl;`
`295`	`295`
Original file line number	Diff line number	Diff line change
`@@ -263,7 +263,7 @@ int cob_loader_impl_destroy(loader_impl impl)`
`263`	`263`	`(void)impl;`
`264`	`264`
`265`	`265`	`// Destroy children loaders`
`266`		`- loader_unload_children(impl);`
	`266`	`+ loader_unload_children(impl, 0);`
`267`	`267`
`268`	`268`	`return cobtidy();`
`269`	`269`	`}`
Original file line number	Diff line number	Diff line change
`@@ -571,7 +571,7 @@ int dart_loader_impl_destroy(loader_impl impl)`
`571`	`571`	`if (dart_impl != nullptr)`
`572`	`572`	`{`
`573`	`573`	`/* Destroy children loaders */`
`574`		`- loader_unload_children(impl);`
	`574`	`+ loader_unload_children(impl, 0);`
`575`	`575`
`576`	`576`	`delete dart_impl;`
`577`	`577`
Original file line number	Diff line number	Diff line change
`@@ -493,7 +493,7 @@ int file_loader_impl_destroy(loader_impl impl)`
`493`	`493`	`if (file_impl != NULL)`
`494`	`494`	`{`
`495`	`495`	`/* Destroy children loaders */`
`496`		`- loader_unload_children(impl);`
	`496`	`+ loader_unload_children(impl, 0);`
`497`	`497`
`498`	`498`	`if (file_impl->execution_paths != NULL)`
`499`	`499`	`{`
Original file line number	Diff line number	Diff line change
`@@ -2023,7 +2023,7 @@ int java_loader_impl_destroy(loader_impl impl)`
`2023`	`2023`	`}`
`2024`	`2024`
`2025`	`2025`	`/* Destroy children loaders */`
`2026`		`- loader_unload_children(impl);`
	`2026`	`+ loader_unload_children(impl, 0);`
`2027`	`2027`
`2028`	`2028`	`java_impl->jvm->DestroyJavaVM();`
`2029`	`2029`