diff --git a/src/runloom_c/mn_sched_runq.c.inc b/src/runloom_c/mn_sched_runq.c.inc index 97135244..fa40424a 100644 --- a/src/runloom_c/mn_sched_runq.c.inc +++ b/src/runloom_c/mn_sched_runq.c.inc @@ -414,6 +414,12 @@ static long long runloom_sysmon_tick_base_ns = 10LL * 1000000LL; /* unbacked-off static runloom_thread_t runloom_sysmon_thread; static int runloom_sysmon_running = 0; /* 1 once the thread is spawned */ static volatile int runloom_sysmon_stop = 0; +/* Interruptible inter-tick wait: stop_join signals the cond under the lock so + * the join returns at once instead of waiting out the watchdog's current tick + * (up to wedge_ns/2 = 25 ms once idle backoff has kicked in -- i.e. at the end + * of every run()). Initialised per spawn, destroyed after the join. */ +static runloom_mutex_t runloom_sysmon_lock; +static runloom_cond_t runloom_sysmon_cond; /* ---- ATTACHED/CPU preemption (RUNLOOM_PREEMPT, default OFF) ---- * diff --git a/src/runloom_c/mn_sched_sysmon.c.inc b/src/runloom_c/mn_sched_sysmon.c.inc index 3e63c522..22d54078 100644 --- a/src/runloom_c/mn_sched_sysmon.c.inc +++ b/src/runloom_c/mn_sched_sysmon.c.inc @@ -152,7 +152,15 @@ static RUNLOOM_THREAD_RET runloom_sysmon_main(void *arg) runloom_sysmon_tick_ns = runloom_sysmon_tick_base_ns << (idle_streak < 3 ? (int)idle_streak : 3); if (cap > 0 && runloom_sysmon_tick_ns > cap) runloom_sysmon_tick_ns = cap; } - runloom_sleep_ns(runloom_sysmon_tick_ns); + /* Wait out the tick on the cond, not a bare sleep, so stop_join can cut + * it short. The stop re-check sits under the same lock stop_join sets + * the flag under, so a stop between the loop test and the wait cannot + * lose its signal. A spurious or early return just rescans. */ + runloom_mutex_lock(&runloom_sysmon_lock); + if (!__atomic_load_n(&runloom_sysmon_stop, __ATOMIC_ACQUIRE)) + runloom_cond_timedwait_ns(&runloom_sysmon_cond, &runloom_sysmon_lock, + runloom_sysmon_tick_ns); + runloom_mutex_unlock(&runloom_sysmon_lock); } free(w_seq); free(w_start); @@ -242,11 +250,18 @@ static void runloom_sysmon_spawn(void) { if (!runloom_sysmon_enabled) return; runloom_sysmon_stop = 0; + /* (Re)initialised here rather than statically: no watchdog is alive at this + * point (the previous one was joined, or died with a fork), and a fork + * child may have inherited the lock held. */ + runloom_mutex_init(&runloom_sysmon_lock); + runloom_cond_init(&runloom_sysmon_cond); if (runloom_thread_create(&runloom_sysmon_thread, runloom_sysmon_main, NULL) == 0) { runloom_sysmon_running = 1; } else { fprintf(stderr, "[RUNLOOM_SYSMON] watchdog thread spawn failed; " "stall detection disabled\n"); + runloom_cond_destroy(&runloom_sysmon_cond); + runloom_mutex_destroy(&runloom_sysmon_lock); } } @@ -255,7 +270,15 @@ static void runloom_sysmon_spawn(void) static void runloom_sysmon_stop_join(void) { if (!runloom_sysmon_running) return; + /* Set + signal under the lock the watchdog re-checks the flag under (see + * the wait at the bottom of runloom_sysmon_main), so the join below + * returns promptly instead of after the watchdog's current tick. */ + runloom_mutex_lock(&runloom_sysmon_lock); __atomic_store_n(&runloom_sysmon_stop, 1, __ATOMIC_RELEASE); + runloom_cond_signal(&runloom_sysmon_cond); + runloom_mutex_unlock(&runloom_sysmon_lock); runloom_thread_join(runloom_sysmon_thread); runloom_sysmon_running = 0; + runloom_cond_destroy(&runloom_sysmon_cond); + runloom_mutex_destroy(&runloom_sysmon_lock); } diff --git a/tests/test_mn_teardown.py b/tests/test_mn_teardown.py index 322880f0..5ae2985a 100644 --- a/tests/test_mn_teardown.py +++ b/tests/test_mn_teardown.py @@ -55,3 +55,56 @@ def main(): runloom.run((i % 4) + 1, main) assert box[0] == 1, i + + +# mn_fini stops the sysmon watchdog first. The watchdog used to wait out each +# tick in a bare sleep, so the join waited for the rest of it -- and by the end +# of a run the hubs are idle, the idle backoff has stretched the tick to its cap +# (wedge_ns/2, so 25 ms by default), and every runloom.run() paid ~20 ms of pure +# teardown latency. With RUNLOOM_SYSMON_MS=2000 the backed-off tick is 80 ms, +# which makes the old stall unmistakable next to a prompt join (~1 ms). The +# watchdog's ticks are phase-locked to mn_init, so a fixed idle time lands at a +# fixed point in the tick: sweep the idle time across one full tick and average, +# else the unfixed stall can hide (a fixed 0.3 s idle measured ~7 ms, not ~40). +_FINI_SNIPPET = r""" +import statistics, time +import runloom, runloom_c + +def one_cycle(idle_s): + runloom_c.mn_init(4) + states = [] + def main(): + states.extend(runloom_c.mn_hub_states()) + runloom.sleep(idle_s) # idle long enough for the backoff to cap + runloom_c.mn_fiber(main) + runloom_c.mn_run() + t0 = time.perf_counter() + runloom_c.mn_fini() + dt = time.perf_counter() - t0 + assert states and all(s["instrumented"] for s in states), states + return dt + +# 0.30 .. 0.38 s in 10 ms steps: one full 80 ms backed-off tick. +print("FINI_MS", statistics.mean(one_cycle(0.30 + 0.01 * k) for k in range(9)) * 1e3) +""" + + +def test_fini_does_not_wait_out_the_sysmon_tick(): + import os + import re + import subprocess + import sys + env = dict(os.environ) + env["PYTHONPATH"] = "src" + env.setdefault("PYTHON_GIL", "0") + env["RUNLOOM_SYSMON"] = "1" # watchdog on even where preempt is off + env["RUNLOOM_SYSMON_QUIET"] = "1" + env["RUNLOOM_SYSMON_MS"] = "2000" # backed-off tick = 80 ms + p = subprocess.run([sys.executable, "-c", _FINI_SNIPPET], env=env, + capture_output=True, text=True, timeout=120) + out = p.stdout + p.stderr + assert p.returncode == 0, out + fini_ms = float(re.search(r"FINI_MS (\S+)", out).group(1)) + # Old behaviour: mean ~40 ms (half the 80 ms tick). Generous bound so a + # loaded CI box does not flake; still far below the unfixed stall. + assert fini_ms < 20.0, "mn_fini took %.1f ms -- waiting out the sysmon tick?\n%s" % (fini_ms, out)