Skip to content

Commit

Permalink
GD-598: Fixing memory leaks on test execution (#599)
Browse files Browse the repository at this point in the history
# Why
Running tests via CMD line shows a lot of memory leaks at program exit.

# What
- fixed releasing of all singletons
- `func_assert` has problems with lambdas when using inside
`timer.timeout.connect`, converted lambda into function
- Handling of stored asserts in the thread context results in
inconsistency during cleanup
- fixed cleanup on `GdUnitCommandHandlerTest`


## Finally, I have reduced most of the memory leaks nodes, but there are
still `GDScriptFunctionState` orphaned nodes that I can't solve because
these are handled internally by Godot see
[74449](godotengine/godot#74449)

```
Statistics: | 947 tests cases | 0 error | 0 failed | 1 flaky | 13 skipped | 0 orphans |

Executed test suites: (100/104), 4 skipped
Executed test cases: (934/947), 13 skipped
Total time:        3min 0s 409ms
Open Report at: file:///home/runner/work/gdUnit4/gdUnit4/reports/report_1/index.html
Exit code: 0
----------------------------------------------------------------
Cleanup singletons ["GdUnitThreadManager", "GdUnitDefaultValueDecoders", "GdUnitCommandHandler"]

	Unregister singleton 'GdUnitThreadManager'
	Free singleton instance 'GdUnitThreadManager:<Object#507678557642>'
	Successfully freed 'GdUnitThreadManager'

	Unregister singleton 'GdUnitDefaultValueDecoders'
	Free singleton instance 'GdUnitDefaultValueDecoders:<Object#509540832395>'
	Successfully freed 'GdUnitDefaultValueDecoders'

	Unregister singleton 'GdUnitCommandHandler'
	Free singleton instance 'GdUnitCommandHandler:<Object#3858659025580>'
	Successfully freed 'GdUnitCommandHandler'
----------------------------------------------------------------
Finallize .. done
Finallize ..
-Orphan nodes report-----------------------
Finallize .. done
XR: Clearing primary interface
XR: Removed interface "Native mobile"
XR: Removed interface "OpenXR"
WARNING: ObjectDB instances leaked at exit (run with --verbose for details).
     at: cleanup (core/object/object.cpp:2327)
Leaked instance: GDScriptFunctionState:9223373291404658414
Leaked instance: GDScriptFunctionState:92233732947[6010](https://github.com/MikeSchulze/gdUnit4/actions/runs/11956449331/job/33331231410?pr=599#step:4:6018)1646
Leaked instance: GDScriptFunctionState:9223373292545509197
Leaked instance: GDScriptFunctionState:9223373291673093970
Leaked instance: GDScriptFunctionState:9223373293719914330
Leaked instance: GDScriptFunctionState:9223373292881053539
Leaked instance: GDScriptFunctionState:9223373296521709422
Leaked instance: GDScriptFunctionState:9223378085393468557
Leaked instance: GDScriptFunctionState:9223382060754801221
Leaked instance: GDScriptFunctionState:9223373293753472331
Leaked instance: GDScriptFunctionState:9223377142497156399
Leaked instance: GDScriptFunctionState:9223378085359924447
Leaked instance: GDScriptFunctionState:9223382060788375283
Hint: Leaked instances typically happen when nodes are removed from the scene tree (with `remove_child()`) but not freed (with `free()` or `queue_free()`).
Orphan StringName: test_case1 (static: 0, total: 1)
Orphan StringName: _validate_callback (static: 0, total: 1)
Orphan StringName: await_millis (static: 0, total: 5)
Orphan StringName: test_timeout_single_yield_wait (static: 0, total: 1)
Orphan StringName: test_timeout_2s (static: 0, total: 1)
Orphan StringName: _execute (static: 0, total: 1)
Orphan StringName: test_timeout_4s (static: 0, total: 1)
Orphan StringName: timeout (static: 2, total: 8)
Orphan StringName: cb_is_equal (static: 0, total: 2)
Orphan StringName: test_timeout_long_running_test_abort (static: 0, total: 1)
Orphan StringName: test_timeout_and_assert_fails (static: 0, total: 1)
StringName: 11 unclaimed string names at exit.
Run tests ends with 0
```
  • Loading branch information
MikeSchulze authored Nov 21, 2024
1 parent d5aa5a8 commit 5a745d4
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 24 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ci-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ jobs:
version: installed
paths: |
res://addons/gdUnit4/test/
arguments: |
--verbose
timeout: 10
publish-report: false
report-name: gdUnit4_Godot${{ matrix.godot-version }}-${{ matrix.godot-status }}
Expand Down
30 changes: 18 additions & 12 deletions addons/gdUnit4/src/asserts/GdUnitFuncAssertImpl.gd
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,19 @@ func _init(instance :Object, func_name :String, args := Array()) -> void:
_current_value_provider = CallBackValueProvider.new(instance, func_name, args)


func _notification(_what :int) -> void:
if is_instance_valid(_current_value_provider):
_current_value_provider.dispose()
_current_value_provider = null
if is_instance_valid(_sleep_timer):
(Engine.get_main_loop() as SceneTree).root.remove_child(_sleep_timer)
_sleep_timer.stop()
_sleep_timer.free()
_sleep_timer = null
func _notification(what :int) -> void:
if what == NOTIFICATION_PREDELETE:
_interrupted = true
var main_node :Node = (Engine.get_main_loop() as SceneTree).root
if is_instance_valid(_current_value_provider):
_current_value_provider.dispose()
_current_value_provider = null
if is_instance_valid(_sleep_timer):
_sleep_timer.set_wait_time(0.0001)
_sleep_timer.stop()
main_node.remove_child(_sleep_timer)
_sleep_timer.free()
_sleep_timer = null


func report_success() -> GdUnitFuncAssert:
Expand Down Expand Up @@ -114,6 +118,10 @@ func cb_is_equal(c :Variant, e :Variant) -> bool: return GdObjects.equals(c,e)
func cb_is_not_equal(c :Variant, e :Variant) -> bool: return not GdObjects.equals(c, e)


func do_interrupt() -> void:
_interrupted = true


func _validate_callback(predicate :Callable, expected :Variant = null) -> void:
if _interrupted:
return
Expand All @@ -125,9 +133,7 @@ func _validate_callback(predicate :Callable, expected :Variant = null) -> void:
scene_tree.root.add_child(timer)
timer.add_to_group("GdUnitTimers")
@warning_ignore("return_value_discarded")
timer.timeout.connect(func do_interrupt() -> void:
_interrupted = true
, CONNECT_DEFERRED)
timer.timeout.connect(do_interrupt, CONNECT_DEFERRED)
timer.set_one_shot(true)
timer.start((_timeout/1000.0)*time_scale)
_sleep_timer = Timer.new()
Expand Down
8 changes: 8 additions & 0 deletions addons/gdUnit4/src/asserts/GdUnitSignalAssertImpl.gd
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ func _init(emitter :Object) -> void:
GdAssertReports.reset_last_error_line_number()


func _notification(what :int) -> void:
if what == NOTIFICATION_PREDELETE:
_interrupted = true
if is_instance_valid(_emitter):
_signal_collector.unregister_emitter(_emitter)
_emitter = null


func report_success() -> GdUnitAssert:
GdAssertReports.report_success()
return self
Expand Down
4 changes: 2 additions & 2 deletions addons/gdUnit4/src/core/GdUnitSingleton.gd
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ static func unregister(p_singleton :String, use_call_deferred :bool = false) ->

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())
var singletons :PackedStringArray = Engine.get_meta(MEATA_KEY, PackedStringArray())
GdUnitTools.prints_verbose("----------------------------------------------------------------")
GdUnitTools.prints_verbose("Cleanup singletons %s" % singletons)
for singleton in singletons:
for singleton in PackedStringArray(singletons):
unregister(singleton, use_call_deferred)
Engine.remove_meta(MEATA_KEY)
GdUnitTools.prints_verbose("----------------------------------------------------------------")
2 changes: 2 additions & 0 deletions addons/gdUnit4/src/core/execution/GdUnitExecutionContext.gd
Original file line number Diff line number Diff line change
Expand Up @@ -348,5 +348,7 @@ func register_auto_free(obj: Variant) -> Variant:

## Runs the gdunit garbage collector to free registered object
func gc() -> void:
# unreference last used assert form the test to prevent memory leaks
GdUnitThreadManager.get_current_context().clear_assert()
await _memory_observer.gc()
orphan_monitor_stop()
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ func _execute(context: GdUnitExecutionContext) -> void:
if _call_stage:
@warning_ignore("redundant_await")
await test_suite.after_test()
# unreference last used assert form the test to prevent memory leaks
GdUnitThreadManager.get_current_context().set_assert(null)

await context.gc()
await context.error_monitor_stop()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ func _execute(context :GdUnitExecutionContext) -> void:

@warning_ignore("redundant_await")
await test_suite.after()
# unreference last used assert form the test to prevent memory leaks
GdUnitThreadManager.get_current_context().set_assert(null)
await context.gc()
var reports := context.build_reports(false)
fire_event(GdUnitEvent.new()\
Expand Down
13 changes: 9 additions & 4 deletions addons/gdUnit4/src/core/thread/GdUnitThreadContext.gd
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ extends RefCounted
var _thread :Thread
var _thread_name :String
var _thread_id :int
var _assert :GdUnitAssert
var _signal_collector :GdUnitSignalCollector
var _execution_context :GdUnitExecutionContext
var _asserts := []


func _init(thread :Thread = null) -> void:
Expand All @@ -21,20 +21,25 @@ func _init(thread :Thread = null) -> void:


func dispose() -> void:
_assert = null
clear_assert()
if is_instance_valid(_signal_collector):
_signal_collector.clear()
_signal_collector = null
_execution_context = null
_thread = null


func clear_assert() -> void:
_asserts.clear()


func set_assert(value :GdUnitAssert) -> void:
_assert = value
if value != null:
_asserts.append(value)


func get_assert() -> GdUnitAssert:
return _assert
return null if _asserts.is_empty() else _asserts[-1]


func set_execution_context(context :GdUnitExecutionContext) -> void:
Expand Down
3 changes: 1 addition & 2 deletions addons/gdUnit4/test/core/command/GdUnitCommandHandlerTest.gd
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ func before() -> void:


func after() -> void:
_handler._notification(NOTIFICATION_PREDELETE)
_handler = null
_handler.free()


@warning_ignore('unused_parameter')
Expand Down

0 comments on commit 5a745d4

Please sign in to comment.