Skip to content

Avoid async IPC panic on peer close - #64142

Open
Jake Bailey (jakebailey) wants to merge 7 commits into
microsoft:mainfrom
jakebailey:fix-async-ipc-peer-close
Open

Avoid async IPC panic on peer close#64142
Jake Bailey (jakebailey) wants to merge 7 commits into
microsoft:mainfrom
jakebailey:fix-async-ipc-peer-close

Conversation

@jakebailey

Copy link
Copy Markdown
Member

Track in-flight request handlers and propagate response write failures through Run instead of panicking from detached goroutines. Cancel handlers during connection shutdown and preserve concurrent errors.
Copilot AI balanced review requested due to automatic review settings September 3, 2026 04:35
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Sep 3, 2026
@typescript-automation typescript-automation Bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Sep 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The critical unbounded teardown wait must be fixed and covered by a regression test.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Prevents async IPC response-write panics after peer closure, but introduces potentially indefinite teardown blocking.

Changes:

  • Propagates response-write failures through Run.
  • Adds a peer-close regression test.
File summaries
File Description
tsc/internal/ipc/conn_async.go Tracks handlers, but synchronously waiting can block teardown indefinitely.
tsc/internal/ipc/conn_async_test.go Tests peer closure, but does not cover a handler remaining blocked.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread tsc/internal/ipc/conn_async.go Outdated
Report completed request errors without synchronously draining request handlers during connection teardown. Cover peer closure while a handler remains blocked.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The terminal error must include the request-handler failure before pending calls are closed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread tsc/internal/ipc/conn_async.go Outdated
Join a completed request-handler failure before recording the connection's terminal error so later calls observe the cause that closed the transport.
Update connection terminal state when a request response write fails, including after Run has returned, so later calls retain the transport failure.
Record response-write failures in terminal connection state before returning from request handling, eliminating the observer race found during review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The peer-close race is handled safely and covered by regression tests.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

2 participants