Skip to content

test(pty): make native wait cleanup cancellation deterministic - #1221

Merged
code-yeongyu merged 1 commit into
mainfrom
fix/pty-threadpool-starvation-macos
Aug 30, 2026
Merged

test(pty): make native wait cleanup cancellation deterministic#1221
code-yeongyu merged 1 commit into
mainfrom
fix/pty-threadpool-starvation-macos

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • mark native PTY sessions cancelled before cleanup signalling in the threadpool starvation fixture
  • avoid a fast SIGKILL exit racing the cancellation flag observation

Root cause

The fixture sent SIGKILL through kill(signal) first. That API intentionally sends only the requested signal and does not set the cancellation flag; the follow-up no-argument kill() could lose a race with waitExit() on macOS, producing a normal exit even though the fixture was cleaning up a cancelled session. The flow-control changes altered scheduling enough to expose this existing fixture race deterministically.

Verification

  • focused native wait test: 2 consecutive passes
  • full @earendil-works/pi-pty suite: 9 files, 71 tests passed
  • cargo test -p senpi-pty: 13 tests passed
  • package TypeScript build passed
  • prebuild freshness check was run and reports the existing vendored darwin-arm64 binary differs from a local rebuild; no native artifact was changed

Summary by cubic

Fixes a race in the native PTY test fixture so cancellation is marked before the cleanup signal.

The fixture previously sent SIGKILL first, which didn't set the cancellation flag, so a fast exit could lose the race with waitExit() on macOS. It now marks cancellation with kill() before sending kill("SIGKILL"), making cleanup deterministic.

Written for commit f3afea9. Summary will update on new commits.

Review in cubic

@code-yeongyu
code-yeongyu merged commit 17f65a3 into main Aug 30, 2026
21 checks passed
@code-yeongyu
code-yeongyu deleted the fix/pty-threadpool-starvation-macos branch August 30, 2026 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant