Commit dc1f316
Add start_new_session=True to mpstat Popen to fix perf hook teardown hang
Summary:
The perf hook's `after_job` iterates `AVAIL_MONITORS` in dict order — and `mpstat` is first. `Monitor.terminate()` (D107426991) now does `os.killpg(os.getpgid(self.proc.pid), signal.SIGINT)` to clean up the bash wrapper + perf-stat child cleanly. But when the Popen does NOT pass `start_new_session=True`, the child inherits the PARENT's process group (= benchpress's own group). The killpg then sends SIGINT to **benchpress itself**, raising KeyboardInterrupt inside `terminate()`, aborting cleanup before any subsequent monitor runs, and before `write_csv()` ever fires.
Symptom: Run A completes the benchmark (metrics.json + breakdown.csv + perf.data written) but the perf hook produces no CSVs (perf-stat.csv, amd-zen4-perf-collector.csv, etc.). benchpress eventually hits the outer `timeout 3600` and exits with `KeyboardInterrupt` traceback ending at `perf_monitors/__init__.py:115` (the `self.proc.wait(timeout=15)` line).
Other monitors that use `subprocess.Popen` (perfstat, topdown's main collectors at lines 187/277/346/505) already have `start_new_session=True` from D107426991. Thread-based monitors (cpufreq_*, memstat, netstat, power, vmstat) override `terminate()` themselves and bypass the killpg path. Only mpstat was unfixed.
Differential Revision: D1076028591 parent bb5440a commit dc1f316
1 file changed
Lines changed: 11 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
22 | 32 | | |
23 | 33 | | |
24 | 34 | | |
| |||
0 commit comments