Revert "terminal: Actually close process groups when the terminal is closed (#61467)" - #62399
Merged
SomeoneToIgnore merged 1 commit intoAug 10, 2026
Merged
Conversation
…closed (zed-industries#61467)" This reverts commit 6297c88.
cla-bot
Bot
added
the
cla-signed
zed-community-bot
Bot
added
the
staff
SomeoneToIgnore
changed the title
SomeoneToIgnore
approved these changes
Aug 10, 2026
Contributor
|
/cherry-pick preview |
zed-zippy Bot
added a commit
that referenced
this pull request
Aug 10, 2026
…closed (#61467)" (#62399) (cherry-pick to preview) (#62412) Cherry-pick of #62399 to preview ---- This reverts commit 6297c88. --- fixes #62286 fixes #62095 #61467 fixed its intended bug, but at the same time introduced an issue where running tasks that would cause new tasks to be terminated immediately. #62322 tried to fix that forward, but was unsuccessful. In the mean-time I am going to revert the original PR. We can try to re-land the original bugfix in a future PR. Release Notes: - N/A Co-authored-by: Chris Biscardi <chris@christopherbiscardi.com>
zed-zippy Bot
added a commit
that referenced
this pull request
Aug 10, 2026
…closed (#61467)" (#62399) (cherry-pick to stable) (#62413) Cherry-pick of #62399 to stable ---- This reverts commit 6297c88. --- fixes #62286 fixes #62095 #61467 fixed its intended bug, but at the same time introduced an issue where running tasks that would cause new tasks to be terminated immediately. #62322 tried to fix that forward, but was unsuccessful. In the mean-time I am going to revert the original PR. We can try to re-land the original bugfix in a future PR. Release Notes: - N/A Co-authored-by: Chris Biscardi <chris@christopherbiscardi.com>
audivir
pushed a commit
to audivir/zed
that referenced
this pull request
Aug 10, 2026
…closed (zed-industries#61467)" (zed-industries#62399) This reverts commit 6297c88. --- fixes zed-industries#62286 fixes zed-industries#62095 zed-industries#61467 fixed its intended bug, but at the same time introduced an issue where running tasks that would cause new tasks to be terminated immediately. zed-industries#62322 tried to fix that forward, but was unsuccessful. In the mean-time I am going to revert the original PR. We can try to re-land the original bugfix in a future PR. Release Notes: - N/A
feitreim
added a commit
to feitreim/zed
that referenced
this pull request
Aug 13, 2026
…n tasks Re-lands zed-industries#61467 (reverted in zed-industries#62399) with the races that caused zed-industries#62095 and zed-industries#62286 designed out. Why the original landing regressed rerun tasks: ProcessIdGetter kept a raw fd number for the PTY master, but the event loop owns that fd and closes it as soon as the child exits - long before the completed task's Terminal entity is dropped. Rerunning a task spawns the replacement terminal first, whose PTY recycles the freed fd number; when the old entity was then dropped, tcgetpgrp on the recycled number read the new terminal's foreground process group and SIGTERM/SIGKILLed it. Why the tcgetsid guard (zed-industries#62322) was not enough: it validated a descriptor Zed does not own with non-atomic syscalls (the fd can be closed or reused between the check and the use), and when the guard rejected, cleanup still fell back to killpg on the child pid captured arbitrarily long ago, signalling a possibly recycled id unconditionally. The re-land replaces both heuristics with two deterministic mechanisms: * ProcessIdGetter now owns a dup of the PTY master (Arc<OwnedFd>), so the descriptor always refers to this terminal's PTY and fd-number recycling cannot occur by construction. Once the session dies, tcgetpgrp on our own master returns 0, so a completed terminal yields no foreground candidate at all. * Process-group ids are validated against the terminal's session before any killpg: the spawned child called setsid, so its pid doubles as the session id, and a group is only signalled while getsid(pgid) still reports that session. A completed terminal therefore captures nothing (its drop is signal-free), and a recycled pid lives in a different session and is rejected. Validation happens once at capture time so the SIGKILL escalation still reaches groups whose leader the SIGTERM already killed. kill_child_process and kill_current_process's stale-fallback path (kill task) get the same session-leader guard. Fixes zed-industries#47412
olejorgenb
added a commit
to olejorgenb/zed
that referenced
this pull request
Aug 19, 2026
Three fixes to `script/find-release-commit`: - Under `set -euo pipefail`, the `match=$(git log ... | grep ...)` assignment aborted the whole script the first time a PR number wasn't found on main, so it exited 1 without printing anything -- not even the merge-base fallback it was supposed to reach. Guard the assignment with `|| true`, which also covers a SIGPIPE from `head` closing the pipe. - A subject can carry more than one PR number, and which one lives on main differs by case: for `foo (zed-industries#61199) (cherry-pick to preview) (zed-industries#62869)` only zed-industries#61199 is on main, while for `Revert "foo (zed-industries#61467)" (zed-industries#62399)` it's zed-industries#62399 -- zed-industries#61467 is the far older commit being undone. Taking the first match traced reverts to the wrong commit; try the numbers right-to-left instead. - When the fallback does kick in, list the commits that couldn't be traced (capped at `UNTRACED_LIMIT`, default 20) so the warning explains itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
playdohface
pushed a commit
to playdohface/zed
that referenced
this pull request
Aug 29, 2026
…closed (zed-industries#61467)" (zed-industries#62399) This reverts commit 6297c88. --- fixes zed-industries#62286 fixes zed-industries#62095 zed-industries#61467 fixed its intended bug, but at the same time introduced an issue where running tasks that would cause new tasks to be terminated immediately. zed-industries#62322 tried to fix that forward, but was unsuccessful. In the mean-time I am going to revert the original PR. We can try to re-land the original bugfix in a future PR. Release Notes: - N/A
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit 6297c88.
fixes #62286
fixes #62095
#61467 fixed its intended bug, but at the same time introduced an issue where running tasks that would cause new tasks to be terminated immediately. #62322 tried to fix that forward, but was unsuccessful. In the mean-time I am going to revert the original PR.
We can try to re-land the original bugfix in a future PR.
Release Notes: