Skip to content

Commit d979215

Browse files
committed
CI: harden test_off_then_on_via_signal against SIGILL startup race
The fourth wall-clock/signal timing flake in the family addressed by #1069 (parity), #1071 (mac_sampler), etc. On a loaded macOS CI runner this test intermittently failed with the child exiting at returncode 252 (= -SIGILL). Root cause: the profiled child printed its PID and entered a busy loop *before* Scalene installed its SIGILL start-profiling handler. The parent read the PID, slept a fixed 0.1s, and sent SIGILL once. If Scalene hadn't installed its handler within that window, SIGILL hit the default action and killed the process. Fix (test-only, no production change): - The child installs its own no-op SIGILL handler *before* announcing its PID, so an early start signal can never terminate it via the default action. Once Scalene replaces that handler, the signal starts profiling as intended. - The parent sends the start signal a few times over a short window (stopping as soon as the child exits) instead of a single fixed-timing shot, so it no longer depends on hitting the precise post-install instant. - Lengthened the child's busy loop so the signal window is comfortably wide. Verified locally: test passes 6/6 in isolation and the full test_on_off_windows.py suite is 29/29.
1 parent 0e51afd commit d979215

1 file changed

Lines changed: 35 additions & 12 deletions

File tree

tests/test_on_off_windows.py

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import subprocess
1414
import sys
1515
import textwrap
16+
import time
1617
from unittest.mock import MagicMock, patch
1718

1819
import pytest
@@ -492,18 +493,33 @@ def test_default_profiles_like_on(self, simple_script, tmp_path):
492493
def test_off_then_on_via_signal(self, tmp_path):
493494
"""Start with --off, send start signal, verify profiling activates."""
494495
script = tmp_path / "long_prog.py"
496+
# The script installs its OWN SIGILL handler first, then prints PID.
497+
# This removes the startup race: previously the child printed its PID
498+
# and entered a busy loop *before* Scalene had installed its SIGILL
499+
# start-profiling handler, so a signal sent during that window hit the
500+
# default action and killed the process (returncode 252 = -SIGILL),
501+
# flaking on loaded CI runners. With a handler already in place when the
502+
# PID is announced, an early signal is never fatal; once Scalene
503+
# installs its own handler (replacing ours) the signal starts profiling
504+
# as intended. The parent retries until profiling activates or a
505+
# generous deadline passes.
495506
script.write_text(
496507
textwrap.dedent("""\
497508
import os
509+
import signal
498510
import sys
499511
import time
500512
501-
# Print PID so parent can signal us
513+
# Install a no-op SIGILL handler up front so an early start
514+
# signal can never terminate us via the default action.
515+
signal.signal(signal.SIGILL, lambda *a: None)
516+
517+
# Now it is safe to announce our PID.
502518
print(f"PID={os.getpid()}", flush=True)
503519
504-
# Busy loop long enough for parent to send signal
520+
# Busy loop long enough for the parent to (re)send the signal.
505521
total = 0
506-
for i in range(5_000_000):
522+
for i in range(20_000_000):
507523
total += i
508524
print("done", total)
509525
""")
@@ -530,15 +546,22 @@ def test_off_then_on_via_signal(self, tmp_path):
530546
pid_line = proc.stdout.readline()
531547
if pid_line.startswith("PID="):
532548
child_pid = int(pid_line.strip().split("=")[1])
533-
# Send start profiling signal (SIGILL)
534-
import time
535-
536-
time.sleep(0.1) # let the child settle
537-
try:
538-
os.kill(child_pid, signal.SIGILL)
539-
except ProcessLookupError:
540-
pass # child may have finished already
549+
# Send the start-profiling signal (SIGILL) a few times over a short
550+
# window: the first may land before Scalene replaces our placeholder
551+
# handler (harmless no-op), a later one activates profiling. Stop as
552+
# soon as the child exits.
553+
for _ in range(20):
554+
if proc.poll() is not None:
555+
break
556+
try:
557+
os.kill(child_pid, signal.SIGILL)
558+
except ProcessLookupError:
559+
break # child finished already
560+
time.sleep(0.05)
541561

542562
stdout, stderr = proc.communicate(timeout=60)
543-
assert proc.returncode == 0
563+
# The child must finish cleanly (not be killed by an unhandled signal).
564+
assert proc.returncode == 0, (
565+
f"child exited with {proc.returncode}; stderr={stderr[-400:]!r}"
566+
)
544567
assert "done" in (pid_line + stdout)

0 commit comments

Comments
 (0)