Skip to content

Revert "test/testenv: run dial-stdio with Pdeathsig and process group"#981

Closed
invidian wants to merge 1 commit intomainfrom
revert-980-fix-leaking-dial-stdio
Closed

Revert "test/testenv: run dial-stdio with Pdeathsig and process group"#981
invidian wants to merge 1 commit intomainfrom
revert-980-fix-leaking-dial-stdio

Conversation

@invidian
Copy link
Copy Markdown
Contributor

@invidian invidian commented Mar 2, 2026

Reverts #980

I am actually still working on a better fix for that, I forgot to turn the original PR into a draft before merging, since it is not cross-platform compatible for example.

Copilot AI review requested due to automatic review settings March 2, 2026 18:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Reverts the earlier change that ran dial-stdio with Pdeathsig and a separate process group, restoring a more cross-platform-friendly implementation.

Changes:

  • Removed syscall.SysProcAttr usage (Setpgid, Pdeathsig) and the associated runtime.LockOSThread() workaround.
  • Simplified process start/wait flow for the docker buildx dial-stdio command.
  • Renamed the process completion channel used by the cleanup select.

Comment thread test/testenv/buildx.go
Comment thread test/testenv/buildx.go
Comment thread test/testenv/buildx.go
Comment thread test/testenv/buildx.go
This reverts commit 5fa6484.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@invidian invidian force-pushed the revert-980-fix-leaking-dial-stdio branch from 22d4cdc to 75972a8 Compare March 3, 2026 06:52
@cpuguy83
Copy link
Copy Markdown
Collaborator

cpuguy83 commented Mar 3, 2026

Handed in #982

@cpuguy83 cpuguy83 closed this Mar 3, 2026
@invidian invidian deleted the revert-980-fix-leaking-dial-stdio branch March 4, 2026 06:26
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.

3 participants