Skip to content

Race in PseudoTerminal handle close vs MonitorExitAsync wait #39

Description

@AThraen

Summary

In PseudoTerminal, MonitorExitAsync blocks on WaitForSingleObject(_hProcess, INFINITE) while Dispose() calls CloseHandle(_hProcess) and zeros the field. Closing a handle while another thread waits on it is Win32 undefined behaviour — WaitForSingleObject can return prematurely, after which Exited?.Invoke() fires even though the child process may not have actually exited.

Where it bites

MainWindow.DisposeAndWaitForExitAsync (introduced in PR #35) subscribes to pty.Exited and then calls vm.Dispose(), expecting the Exited event to mean "the child has exited so its ~/.claude.json write is done." With the race, the event can fire on handle-close instead, and the next claude teardown can begin before the previous one has flushed.

Practical impact today is bounded by:

  • Dispose() also calls ClosePseudoConsole(_hPC), which signals the child to shut down, so it usually exits promptly anyway.
  • The caller has a Task.Delay(Math.Min(postExitMs, 1000)) post-exit pause as belt-and-braces.

So the worst-case gap between teardowns is ~1s instead of "real exit + 1s," not zero. But the abstraction is leaky and the rare close-race could still corrupt ~/.claude.json.

Proposed fix

Pick one:

  1. DuplicateHandle for the wait. Duplicate _hProcess into a wait handle before disposing the PTY; MonitorExitAsync waits on the duplicate and the original can be closed independently in Dispose().
  2. PseudoTerminal.WaitForExitAsync(timeoutMs) API. New method that waits on the process handle (kept alive until the wait returns) and only then proceeds with handle cleanup. Dispose() can call this internally if the user didn't.

Either is ~15 lines.

Discovered by

Copilot review on PR #35, validated. Filed as follow-up rather than fixed inline because the practical impact is limited and the fix touches PseudoTerminal's public surface.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions