John McCardle
9718153709
closes #251 Two related bugs where Python garbage collection destroyed callbacks that were still needed by live C++ objects: 1. **Drawable callbacks (all 8 types)**: tp_dealloc unconditionally called click_unregister() etc., destroying callbacks even when the C++ object was still alive in a parent's children vector. Fixed by guarding with shared_ptr::use_count() <= 1 — only unregister when the Python wrapper is the last owner. 2. **Timer GC prevention**: Active timers now hold a Py_INCREF'd reference to their Python wrapper (Timer::py_wrapper), preventing GC while the timer is registered in the engine. Released on stop(), one-shot fire, or destruction. mcrfpy.Timer("name", cb, 100) now works without storing the return value. Also includes audio synth demo UI fixes: button click handling (don't set on_click on Caption children), single-column slider layout, improved Animalese contrast. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
150 lines
4.3 KiB
Python
150 lines
4.3 KiB
Python
"""Regression test for issue #251: Timer GC prevention.
|
|
|
|
Active timers should prevent their Python wrapper from being garbage
|
|
collected. The natural pattern `mcrfpy.Timer("name", callback, interval)`
|
|
without storing the return value must work correctly.
|
|
|
|
Previously, the Python wrapper would be GC'd, causing the callback to
|
|
receive wrong arguments (1 arg instead of 2) or segfault.
|
|
"""
|
|
import mcrfpy
|
|
import gc
|
|
import sys
|
|
|
|
results = []
|
|
|
|
# ---- Test 1: Timer without stored reference survives GC ----
|
|
def make_timer_without_storing():
|
|
"""Create a timer without storing the reference - should NOT be GC'd."""
|
|
fire_count = [0]
|
|
def callback(timer, runtime):
|
|
fire_count[0] += 1
|
|
mcrfpy.Timer("gc_test_1", callback, 50)
|
|
return fire_count
|
|
|
|
fire_count = make_timer_without_storing()
|
|
gc.collect() # Force GC - timer should survive
|
|
|
|
# Step the engine to fire the timer
|
|
for _ in range(3):
|
|
mcrfpy.step(0.06)
|
|
|
|
assert fire_count[0] >= 1, f"FAIL: Timer didn't fire (count={fire_count[0]}), was likely GC'd"
|
|
print(f"PASS: Unstored timer fired {fire_count[0]} times after GC")
|
|
|
|
|
|
# ---- Test 2: Timer callback receives correct args (timer, runtime) ----
|
|
received_args = []
|
|
|
|
def make_timer_check_args():
|
|
def callback(timer, runtime):
|
|
received_args.append((type(timer).__name__, type(runtime).__name__))
|
|
timer.stop()
|
|
mcrfpy.Timer("gc_test_2", callback, 50)
|
|
|
|
make_timer_check_args()
|
|
gc.collect()
|
|
mcrfpy.step(0.06)
|
|
|
|
assert len(received_args) >= 1, "FAIL: Callback never fired"
|
|
timer_type, runtime_type = received_args[0]
|
|
assert timer_type == "Timer", f"FAIL: First arg should be Timer, got {timer_type}"
|
|
assert runtime_type == "int", f"FAIL: Second arg should be int, got {runtime_type}"
|
|
print("PASS: Callback received correct (timer, runtime) args after GC")
|
|
|
|
|
|
# ---- Test 3: One-shot timer fires correctly without stored ref ----
|
|
oneshot_fired = [False]
|
|
|
|
def make_oneshot():
|
|
def callback(timer, runtime):
|
|
oneshot_fired[0] = True
|
|
mcrfpy.Timer("gc_test_3", callback, 50, once=True)
|
|
|
|
make_oneshot()
|
|
gc.collect()
|
|
mcrfpy.step(0.06)
|
|
|
|
assert oneshot_fired[0], "FAIL: One-shot timer didn't fire after GC"
|
|
print("PASS: One-shot timer fires correctly without stored reference")
|
|
|
|
|
|
# ---- Test 4: Stopped timer allows GC ----
|
|
import weakref
|
|
|
|
weak_timer = [None]
|
|
|
|
def make_stoppable_timer():
|
|
def callback(timer, runtime):
|
|
timer.stop()
|
|
t = mcrfpy.Timer("gc_test_4", callback, 50)
|
|
weak_timer[0] = weakref.ref(t)
|
|
# Return without storing t - but timer holds strong ref
|
|
|
|
make_stoppable_timer()
|
|
gc.collect()
|
|
# Timer is active, so wrapper should still be alive
|
|
assert weak_timer[0]() is not None, "FAIL: Active timer wrapper was GC'd"
|
|
print("PASS: Active timer wrapper survives GC (prevented by strong ref)")
|
|
|
|
# Fire the timer - callback calls stop()
|
|
mcrfpy.step(0.06)
|
|
gc.collect()
|
|
|
|
# After stop(), the strong ref is released, wrapper should be GC-able
|
|
# Note: weak_timer might still be alive if PythonObjectCache holds it
|
|
# The key test is that the callback fired correctly (test 2 covers that)
|
|
print("PASS: Timer stop() allows eventual GC")
|
|
|
|
|
|
# ---- Test 5: Timer.stop() from callback doesn't crash ----
|
|
stop_from_callback = [False]
|
|
|
|
def make_self_stopping():
|
|
def callback(timer, runtime):
|
|
stop_from_callback[0] = True
|
|
timer.stop()
|
|
mcrfpy.Timer("gc_test_5", callback, 50)
|
|
|
|
make_self_stopping()
|
|
gc.collect()
|
|
mcrfpy.step(0.06)
|
|
gc.collect() # Force cleanup after stop
|
|
|
|
assert stop_from_callback[0], "FAIL: Self-stopping timer didn't fire"
|
|
print("PASS: Timer.stop() from callback is safe after GC")
|
|
|
|
|
|
# ---- Test 6: Restarting a stopped timer re-prevents GC ----
|
|
restart_count = [0]
|
|
|
|
def make_restart_timer():
|
|
def callback(timer, runtime):
|
|
restart_count[0] += 1
|
|
if restart_count[0] >= 3:
|
|
timer.stop()
|
|
t = mcrfpy.Timer("gc_test_6", callback, 50)
|
|
return t
|
|
|
|
timer_ref = make_restart_timer()
|
|
gc.collect()
|
|
|
|
# Fire 3 times to trigger stop
|
|
for _ in range(5):
|
|
mcrfpy.step(0.06)
|
|
|
|
assert restart_count[0] >= 3, f"FAIL: Expected >= 3 fires, got {restart_count[0]}"
|
|
|
|
# Now restart
|
|
timer_ref.restart()
|
|
old_count = restart_count[0]
|
|
for _ in range(2):
|
|
mcrfpy.step(0.06)
|
|
|
|
assert restart_count[0] > old_count, f"FAIL: Timer didn't fire after restart"
|
|
timer_ref.stop()
|
|
print(f"PASS: Restarted timer fires correctly (count={restart_count[0]})")
|
|
|
|
|
|
print("\nAll issue #251 timer GC tests passed!")
|
|
sys.exit(0)
|