GD-503: Rework plugin exit to avoid system crash and memory leaks (#504)

# Why
The plugin has some issues to release resources at Godot exit and result
sometimes in a segmentation fault under Linux

# What
- fix `remove_child` on `free_instance` to be called deferred now
- removed the temporary `free_fix` workaround
- fix orphan nodes in `GdUnitTestDiscoverer`
- introduce a new flag to call deferred on `free_instance` at plugin
exit


# Godot bug related
godotengine/godot#92727

addons/gdUnit4/plugin.gd

@@ -1,8 +1,8 @@
 @tool
 extends EditorPlugin
 
-const GdUnitTools := preload("res://addons/gdUnit4/src/core/GdUnitTools.gd")
-const GdUnitTestDiscoverGuard := preload("res://addons/gdUnit4/src/core/discovery/GdUnitTestDiscoverGuard.gd")
+const GdUnitTools := preload ("res://addons/gdUnit4/src/core/GdUnitTools.gd")
+const GdUnitTestDiscoverGuard := preload ("res://addons/gdUnit4/src/core/discovery/GdUnitTestDiscoverGuard.gd")
 
 
 var _gd_inspector :Node
@@ -26,7 +26,7 @@ func _enter_tree() -> void:
 	add_control_to_bottom_panel(_gd_console, "gdUnitConsole")
 	prints("Loading GdUnit4 Plugin success")
 	if GdUnitSettings.is_update_notification_enabled():
-		var update_tool :Node = load("res://addons/gdUnit4/src/update/GdUnitUpdateNotify.tscn").instantiate()
+		var update_tool: Node = load("res://addons/gdUnit4/src/update/GdUnitUpdateNotify.tscn").instantiate()
 		Engine.get_main_loop().root.add_child.call_deferred(update_tool)
 	if GdUnit4CSharpApiLoader.is_mono_supported():
 		prints("GdUnit4Net version '%s' loaded." % GdUnit4CSharpApiLoader.version())
@@ -40,11 +40,11 @@ func _exit_tree() -> void:
 		return
 	if is_instance_valid(_gd_inspector):
 		remove_control_from_docks(_gd_inspector)
-		GodotVersionFixures.free_fix(_gd_inspector)
+		_gd_inspector.free()
 	if is_instance_valid(_gd_console):
 		remove_control_from_bottom_panel(_gd_console)
 		_gd_console.free()
-	GdUnitTools.dispose_all.call_deferred()
+	GdUnitTools.dispose_all(true)
 	prints("Unload GdUnit4 Plugin success")
 
 
@@ -54,6 +54,6 @@ func check_running_in_test_env() -> bool:
 	return DisplayServer.get_name() == "headless" or args.has("--selftest") or args.has("--add") or args.has("-a") or args.has("--quit-after") or args.has("--import")
 
 
-func _on_resource_saved(resource :Resource) -> void:
+func _on_resource_saved(resource: Resource) -> void:
 	if resource is Script:
 		_guard.discover(resource)

addons/gdUnit4/src/core/GdUnitSingleton.gd

@@ -28,26 +28,26 @@ static func instance(name :String, clazz :Callable) -> Variant:
 	return singleton
 
 
-static func unregister(p_singleton :String) -> void:
+static func unregister(p_singleton :String, use_call_deferred :bool = false) -> void:
 	var singletons :PackedStringArray = Engine.get_meta(MEATA_KEY, PackedStringArray())
 	if singletons.has(p_singleton):
 		GdUnitTools.prints_verbose("\n	Unregister singleton '%s'" % p_singleton);
 		var index := singletons.find(p_singleton)
 		singletons.remove_at(index)
 		var instance_ :Object = Engine.get_meta(p_singleton)
 		GdUnitTools.prints_verbose("	Free singleton instance '%s:%s'" % [p_singleton, instance_])
-		GdUnitTools.free_instance(instance_)
+		GdUnitTools.free_instance(instance_, use_call_deferred)
 		Engine.remove_meta(p_singleton)
 		GdUnitTools.prints_verbose("	Successfully freed '%s'" % p_singleton)
 	Engine.set_meta(MEATA_KEY, singletons)
 
 
-static func dispose() -> void:
+static func dispose(use_call_deferred :bool = false) -> void:
 	# use a copy because unregister is modify the singletons array
 	var singletons := PackedStringArray(Engine.get_meta(MEATA_KEY, PackedStringArray()))
 	GdUnitTools.prints_verbose("----------------------------------------------------------------")
 	GdUnitTools.prints_verbose("Cleanup singletons %s" % singletons)
 	for singleton in singletons:
-		unregister(singleton)
+		unregister(singleton, use_call_deferred)
 	Engine.remove_meta(MEATA_KEY)
 	GdUnitTools.prints_verbose("----------------------------------------------------------------")

addons/gdUnit4/src/core/GdUnitTools.gd

@@ -27,7 +27,7 @@ static func prints_verbose(message :String) -> void:
 		prints(message)
 
 
-static func free_instance(instance :Variant, is_stdout_verbose :=false) -> bool:
+static func free_instance(instance :Variant, use_call_deferred :bool = false, is_stdout_verbose := false) -> bool:
 	if instance is Array:
 		for element :Variant in instance:
 			free_instance(element)
@@ -52,15 +52,26 @@ static func free_instance(instance :Variant, is_stdout_verbose :=false) -> bool:
 		#release_connections(instance)
 		if instance is Timer:
 			instance.stop()
-			instance.call_deferred("free")
-			await Engine.get_main_loop().process_frame
+			if use_call_deferred:
+				instance.call_deferred("free")
+			else:
+				instance.free()
+				await Engine.get_main_loop().process_frame
 			return true
 		if instance is Node and instance.get_parent() != null:
 			if is_stdout_verbose:
 				print_verbose("GdUnit4:gc():remove node from parent ",  instance.get_parent(), instance)
-			instance.get_parent().remove_child(instance)
-			instance.set_owner(null)
-		instance.free()
+			if use_call_deferred:
+				instance.get_parent().remove_child.call_deferred(instance)
+				#instance.call_deferred("set_owner", null)
+			else:
+				instance.get_parent().remove_child(instance)
+		if is_stdout_verbose:
+			print_verbose("GdUnit4:gc():freeing `free()` the instance ", instance)
+		if use_call_deferred:
+			instance.call_deferred("free")
+		else:
+			instance.free()
 		return !is_instance_valid(instance)
 
 
@@ -92,9 +103,9 @@ static func release_timers() -> void:
 
 
 # the finally cleaup unfreed resources and singletons
-static func dispose_all() -> void:
+static func dispose_all(use_call_deferred :bool = false) -> void:
 	release_timers()
-	GdUnitSingleton.dispose()
+	GdUnitSingleton.dispose(use_call_deferred)
 	GdUnitSignals.dispose()
 
 

addons/gdUnit4/src/core/GodotVersionFixures.gd

@@ -19,13 +19,3 @@ static func set_event_global_position(event: InputEventMouseMotion, global_posit
 		event.global_position = event.position
 	else:
 		event.global_position = global_position
-
-
-# we crash on macOS when using free() inside the plugin _exit_tree
-static func free_fix(instance: Object) -> void:
-	var distribution_name := OS.get_distribution_name()
-	if distribution_name != "Windows":
-		prints("Using queue_free() hotfix on system:", distribution_name)
-		instance.queue_free()
-	else:
-		instance.free()

addons/gdUnit4/src/core/command/GdUnitCommandHandler.gd

@@ -228,9 +228,10 @@ func cmd_stop(client_id :int) -> void:
 	if _running_debug_mode:
 		EditorInterface.stop_playing_scene()
 	else: if _current_runner_process_id > 0:
-		var result := OS.kill(_current_runner_process_id)
-		if result != OK:
-			push_error("ERROR checked stopping GdUnit Test Runner. error code: %s" % result)
+		if OS.is_process_running(_current_runner_process_id):
+			var result := OS.kill(_current_runner_process_id)
+			if result != OK:
+				push_error("ERROR checked stopping GdUnit Test Runner. error code: %s" % result)
 	_current_runner_process_id = -1
 
 

addons/gdUnit4/src/core/discovery/GdUnitTestDiscoverer.gd

@@ -19,14 +19,16 @@ static func run() -> void:
 
 		# Do sync the main thread before emit the discovered test suites to the inspector
 		await Engine.get_main_loop().process_frame
+		var test_case_count :int = 0
 		for test_suite in _test_suites_to_process:
+			test_case_count += test_suite.get_child_count()
 			var ts_dto := GdUnitTestSuiteDto.of(test_suite)
 			GdUnitSignals.instance().gdunit_add_test_suite.emit(ts_dto)
+			test_suite.free()
 
 		prints("%d test suites discovered." % _test_suites_to_process.size())
-		var test_case_count :int = _test_suites_to_process.reduce(func (accum :int, test_suite :Node) -> int:
-					return accum + test_suite.get_child_count(), 0)
 		GdUnitSignals.instance().gdunit_event.emit(GdUnitEventTestDiscoverEnd.new(_test_suites_to_process.size(), test_case_count))
+		_test_suites_to_process.clear()
 	)
 	# wait unblocked to the tread is finished
 	while t.is_alive():

addons/gdUnit4/src/network/GdUnitServer.tscn

@@ -1,10 +1,10 @@
-[gd_scene load_steps=3 format=2]
+[gd_scene load_steps=3 format=3 uid="uid://cn5mp3tmi2gb1"]
 
-[ext_resource path="res://addons/gdUnit4/src/network/GdUnitServer.gd" type="Script" id=1]
-[ext_resource path="res://addons/gdUnit4/src/network/GdUnitTcpServer.gd" type="Script" id=2]
+[ext_resource type="Script" path="res://addons/gdUnit4/src/network/GdUnitServer.gd" id="1"]
+[ext_resource type="Script" path="res://addons/gdUnit4/src/network/GdUnitTcpServer.gd" id="2"]
 
 [node name="Control" type="Node"]
-script = ExtResource( 1 )
+script = ExtResource("1")
 
 [node name="TcpServer" type="Node" parent="."]
-script = ExtResource( 2 )
+script = ExtResource("2")

addons/gdUnit4/src/network/GdUnitTcpClient.gd

@@ -119,8 +119,8 @@ func rpc_receive() -> RPC:
 	return null
 
 
-func console(message :String) -> void:
-	prints("TCP Client:", message)
+func console(_message :String) -> void:
+	#prints("TCP Client:", _message)
 	pass
 
 

addons/gdUnit4/src/network/GdUnitTcpServer.gd

@@ -30,10 +30,11 @@ class TcpConnection extends Node:
 
 
 	func close() -> void:
-		rpc_send(RPCClientDisconnect.new().with_id(_id))
-		server().client_disconnected.emit(_id)
-		_stream.disconnect_from_host()
-		_readBuffer = ""
+		if _stream != null:
+			_stream.disconnect_from_host()
+			_readBuffer = ""
+			_stream = null
+			queue_free()
 
 
 	func id() -> int:
@@ -44,12 +45,12 @@ class TcpConnection extends Node:
 		return get_parent()
 
 
-	func rpc_send(p_rpc :RPC) -> void:
+	func rpc_send(p_rpc: RPC) -> void:
 		_stream.put_var(p_rpc.serialize(), true)
 
 
-	func _process(_delta :float) -> void:
-		if _stream.get_status() != StreamPeerTCP.STATUS_CONNECTED:
+	func _process(_delta: float) -> void:
+		if _stream == null or _stream.get_status() != StreamPeerTCP.STATUS_CONNECTED:
 			return
 		receive_packages()
 
@@ -71,7 +72,7 @@ class TcpConnection extends Node:
 					server().rpc_data.emit(rpc_)
 
 
-	func _read_next_data_packages(data_package :PackedByteArray) -> PackedStringArray:
+	func _read_next_data_packages(data_package: PackedByteArray) -> PackedStringArray:
 		_readBuffer += data_package.get_string_from_utf8()
 		var json_array := _readBuffer.split(GdUnitServerConstants.JSON_RESPONSE_DELIMITER)
 		# We need to check if the current data is terminated by the delemiter (data packets can be split unspecifically).
@@ -100,7 +101,7 @@ func _ready() -> void:
 	client_disconnected.connect(_on_client_disconnected)
 
 
-func _notification(what :int) -> void:
+func _notification(what: int) -> void:
 	if what == NOTIFICATION_PREDELETE:
 		stop()
 
@@ -131,34 +132,32 @@ func stop() -> void:
 		if connection is TcpConnection:
 			connection.close()
 			remove_child(connection)
+	_server = null
 
 
-func disconnect_client(client_id :int) -> void:
-	for connection in get_children():
-		if connection is TcpConnection and connection.id() == client_id:
-			connection.close()
+func disconnect_client(client_id: int) -> void:
+	client_disconnected.emit(client_id)
 
 
-func _process(_delta :float) -> void:
-	if not _server.is_listening():
+func _process(_delta: float) -> void:
+	if _server != null and not _server.is_listening():
 		return
 	# check if connection is ready to be used
 	if _server.is_connection_available():
 		add_child(TcpConnection.new(_server))
 
 
-func _on_client_connected(client_id :int) -> void:
+func _on_client_connected(client_id: int) -> void:
 	console("Client connected %d" % client_id)
 
 
-func _on_client_disconnected(client_id :int) -> void:
-	console("Client disconnected %d" % client_id)
+func _on_client_disconnected(client_id: int) -> void:
 	for connection in get_children():
 		if connection is TcpConnection and connection.id() == client_id:
+			connection.close()
 			remove_child(connection)
 
 
-
-func console(_message :String) -> void:
+func console(_message: String) -> void:
 	#print_debug("TCP Server:", _message)
 	pass