Terminate child processes on Windows (fix #3134) by pjeby · Pull Request #4723 · junegunn/fzf

pjeby · 2026-03-18T04:52:09Z

Windows doesn't have signals, the default Kill doesn't actually kill anything, and other forms of termination don't allow cleanup. So we spawn processes as their own process groups and then send them a CTRL_BREAK_EVENT to terminate. (If they weren't in a separate group, the Ctrl-Break would trigger a SIGINT-equivalent to the fzf process, i.e. fzf would also terminate itself!)

This won't forcibly terminate misbehaving processes, but it improves the current circumstances for well-behaved (but long-running) preview processes (such as docker logs run against a remote server with huge logs).

Copilot

Pull request overview

This PR improves termination of spawned child processes on Windows by isolating them into their own process groups and attempting to stop them via a console Ctrl-Break event, which enables better cleanup behavior for well-behaved long-running commands (e.g., preview processes).

Changes:

Create child processes with CREATE_NEW_PROCESS_GROUP on Windows.
Switch KillCommand from Process.Kill() to GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, ...).
Add golang.org/x/sys/windows dependency in the Windows utility implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pjeby · 2026-03-18T08:10:33Z

 // KillCommand kills the process for the given command
 func KillCommand(cmd *exec.Cmd) error {
-	return cmd.Process.Kill()
+	return windows.GenerateConsoleCtrlEvent(windows.CTRL_BREAK_EVENT, uint32(cmd.Process.Pid))


I don't know if this makes any sense. The entire reason for this patch is that cmd.process.Kill() does not actually terminate the process on Windows, let alone the process group. So ISTM that adding a fallback to the old behavior wouldn't actually do anything useful here.

I see. Just to confirm, is the limitation documented somewhere?

Here's a comment describing the workaround; other comments above it provide more background. There's a more recent issue that deals with the "Kill doesn't let you finish Wait if I/O is ongoing" aspect of the problem. (Which IIUC is why the existing code still hangs despite the Kill.)

There are also various libraries and apps written in Go that use various workarounds, with comments along the lines that Kill is essentially useless on Windows for one reason or another. (For example, when invoking a shell, killing the shell doesn't necessarily terminate the shell's children, and the Wait hangs because of this.)

(There is also a mechanism that I think got added to allow contexts to more easily kill child processes, but IIUC it still requires you to write a custom process killer - it's mostly that they added a way to carry that information in the context so you could get a notification of needing to kill the process on context cancellation. That issue also brings up the inability to use Signal.Interrupt on Windows, and generally waves off that problem as basically, "well, Windows users can write their own process killers, we're not dealing with that problem here.")

As for the issue at hand, my reason for pushback on the fallback suggestion isn't that it's a bad idea as such, but that it might give the false impression of doing something useful, when it's just falling back to the existing behavior that demonstrably doesn't work. (vs., say falling back to more aggressive methods of killing the process, that do work even when Kill doesn't.) But if it's documented as a FIXME-type stopgap, it would certainly cover the case where the existing method is working for someone, but the new method isn't.

It's mostly that the AI-suggested text implies Kill is a hard-kill, and in practical terms it isn't. Even if were terminating the shell (and I don't think it does), child processes spawned by the shell can definitely be unaffected, and the Wait still hangs. So I'd suggest at least changing the comment to reflect the "well, this probably won't work either, but what the heck, might as well try anyway" nature of the fallback.)

Thanks for the detailed write-up.

Just to clarify, I'm not suggesting that we need this fallback or that it's correct or appropriate, I just wanted to be clear.

I mean I was curious whether this limitation of the Kill on Windows is mentioned on the official documentation of Golang as I couldn't easily find it. Because if it's not mentioned, how would someone know? And if it's not documented, someone might assume it's a bug that could be fixed in a future Go release. From that perspective, introducing a fallback could seem justifiable.

Certainly! Unfortunately the general approach of Go seems to be that APIs have platform-specific limitations and it's up to the developer to figure that out, rather than for the Go team to emulate Posix.

(Of course, they also don't support cygwin/msys as a platform, which would mostly solve this and a ton of similar things by at least allowing Go programs running on Windows to use a shared, standardized, and hardened Posix emulation. So everyone is stuck making their own half-baked wrappers and workarounds instead. ☹️ )

Anyway, happy to add the suggested bit, just think I'd want to tweak the comments a bit to make the limitations clear. And as I mentioned in #3134, there are some hard-kill options that can also be taken if you'd prefer.

everyone is stuck making their own half-baked wrappers and workarounds instead. ☹️

Oh, well.

I'd want to tweak the comments a bit to make the limitations clear.

That would be great!

Done. I also tweaked the logic so the Ctrl-Break is only sent if the process was created with its own group. (There aren't currently any code paths that call Kill on something without its own group, but might as well avoid that possibly becoming a footgun later.)

Windows doesn't have signals the default Kill doesn't actually kill anything, and other forms of termination don't allow cleanup. So we spawn processes as their own process groups and then send them a CTRL_BREAK_EVENT to terminate. (If they weren't in a separate group, the Ctrl-Break would trigger a SIGINT-equivalent to the fzf process, i.e. fzf would also terminate itself.) This won't forcibly terminate misbehaving processes, but it improves the current circumstances for well-behaved (but long-running) preview processes.

`executeCommand` and `Become` never kill their `cmd`, so they can live without a process group, and should participate in console interrupts. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

junegunn

LGTM, approved! I'll wait a day or two to see if any Windows user has something to say.

pjeby · 2026-03-28T12:13:19Z

Ok, this is now updated to only apply to pwsh and unknown/posixy shells, per discussion.

Copilot

Pull request overview

Improves process termination behavior on Windows by optionally creating a new process group for spawned shell commands and attempting to terminate them via CTRL_BREAK_EVENT (better cleanup semantics than Kill), with specific handling for PowerShell 7+ (pwsh).

Changes:

Add explicit detection of pwsh as a distinct shell type.
When setpgid is requested, set CREATE_NEW_PROCESS_GROUP for pwsh and “unknown/posix-ish” shells, and add -NonInteractive for pwsh to ensure it exits on Ctrl-Break.
Update KillCommand to send CTRL_BREAK_EVENT to the child process group when applicable, otherwise fall back to Process.Kill().

Comments suppressed due to low confidence (1)

src/util/util_windows.go:60

NewExecutor only infers shellType when withShell is empty (len(args)==0). When a user supplies --with-shell pwsh ... / powershell ..., shellType stays shellTypeUnknown, which means (1) QuoteEntry will use POSIX-style quoting (likely incorrect for PowerShell) and (2) ExecCommand won't add the pwsh-specific -NonInteractive flag even when setpgid is true. Consider always deriving shellType from filepath.Base(shell) regardless of withShell, and only using the hardcoded default args (/s/c, -NoProfile, etc.) when the user didn't provide any args.

	shellType := shellTypeUnknown
	basename := filepath.Base(shell)
	if len(args) > 0 {
		args = args[1:]
	} else if strings.HasPrefix(basename, "cmd") {
		shellType = shellTypeCmd
		args = []string{"/s/c"}
	} else if strings.HasPrefix(basename, "pwsh") {
		shellType = shellTypePwsh
		args = []string{"-NoProfile", "-Command"}
	} else if strings.HasPrefix(basename, "powershell") {
		shellType = shellTypePowerShell
		args = []string{"-NoProfile", "-Command"}
	} else {
		args = []string{"-c"}
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Junegunn Choi <junegunn.c@gmail.com>

junegunn

LGTM, thanks!

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [junegunn/fzf](https://github.com/junegunn/fzf) | minor | `v0.70.0` → `v0.71.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>junegunn/fzf (junegunn/fzf)</summary> ### [`v0.71.0`](https://github.com/junegunn/fzf/releases/tag/v0.71.0): 0.71.0 [Compare Source](junegunn/fzf@v0.70.0...v0.71.0) *Release highlights: <https://junegunn.github.io/fzf/releases/0.71.0/>* - Added `--popup` as a new name for `--tmux` with Zellij support - `--popup` starts fzf in a tmux popup or a Zellij floating pane - `--tmux` is now an alias for `--popup` - Requires tmux 3.3+ or Zellij 0.44+ - Cross-reload item identity with `--id-nth` - Added `--id-nth=NTH` to define item identity fields for cross-reload operations - When a `reload` is triggered with tracking enabled, fzf searches for the tracked item by its identity fields in the new list. - `--track --id-nth ..` tracks by the entire line - `--track --id-nth 1` tracks by the first field - `--track` without `--id-nth` retains the existing index-based tracking behavior - The UI is temporarily blocked (prompt dimmed, input disabled) until the item is found or loading completes. - Press `Escape` or `Ctrl-C` to cancel the blocked state without quitting - Info line shows `+T*` / `+t*` while searching - With `--multi`, selected items are preserved across `reload-sync` by matching their identity fields - Performance improvements - The search performance now scales linearly with the number of CPU cores, as we dropped static partitioning to allow better load balancing across threads. ``` === query: 'linux' === [all] baseline: 21.95ms current: 17.47ms (1.26x) matches: 179966 (12.79%) [1T] baseline: 179.63ms current: 180.53ms (1.00x) matches: 179966 (12.79%) [2T] baseline: 97.38ms current: 90.05ms (1.08x) matches: 179966 (12.79%) [4T] baseline: 53.83ms current: 44.77ms (1.20x) matches: 179966 (12.79%) [8T] baseline: 41.66ms current: 22.58ms (1.84x) matches: 179966 (12.79%) ``` - Improved the cache structure, reducing memory footprint per entry by 86x. - With the reduced per-entry cost, the cache now has broader coverage. - Shell integration improvements - bash: CTRL-R now supports multi-select and `shift-delete` to delete history entries ([#4715](junegunn/fzf#4715)) - fish: - Improved command history (CTRL-R) ([#4703](junegunn/fzf#4703)) ([@bitraid](https://github.com/bitraid)) - Rewrite completion script (SHIFT-TAB) ([#4731](junegunn/fzf#4731)) ([@bitraid](https://github.com/bitraid)) - Increase minimum fish version requirement to 3.4.0 ([#4731](junegunn/fzf#4731)) ([@bitraid](https://github.com/bitraid)) - `GET /` HTTP endpoint now includes `positions` field in each match entry, providing the indices of matched characters for external highlighting ([#4726](junegunn/fzf#4726)) - Allow adaptive height with negative value (`--height=~-HEIGHT`) ([#4682](junegunn/fzf#4682)) - Bug fixes - `--walker=follow` no longer follows symlinks whose target is an ancestor of the walker root, avoiding severe resource exhaustion when a symlink points outside the tree (e.g. Wine's `z:` → `/`) ([#4710](junegunn/fzf#4710)) - Fixed AWK tokenizer not treating a new line character as whitespace - Fixed `--{accept,with}-nth` removing trailing whitespaces with a non-default `--delimiter` - Fixed OSC8 hyperlinks being mangled when the URL contains unicode characters ([#4707](junegunn/fzf#4707)) - Fixed `--with-shell` not handling quoted arguments correctly ([#4709](junegunn/fzf#4709)) - Fixed child processes not being terminated on Windows ([#4723](junegunn/fzf#4723)) ([@pjeby](https://github.com/pjeby)) - Fixed preview scrollbar not rendered after `toggle-preview` - Fixed preview follow/scroll with long wrapped lines - Fixed tab width when `--frozen-left` is used - Fixed preview mouse events being processed when no preview window exists - zsh: Fixed history widget when `sh_glob` option is on ([#4714](junegunn/fzf#4714)) ([@EvanHahn](https://github.com/EvanHahn)) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

github-actions Bot added the go Go code label Mar 18, 2026

junegunn requested a review from Copilot March 18, 2026 07:48

Copilot started reviewing on behalf of junegunn March 18, 2026 07:48 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

junegunn added the windows Windows label Mar 18, 2026

junegunn mentioned this pull request Mar 19, 2026

FZF should update preview on the new item selection immediately #3134

Closed

5 tasks

pjeby and others added 4 commits March 19, 2026 00:13

Only put killable processes in a new group

542881d

`executeCommand` and `Become` never kill their `cmd`, so they can live without a process group, and should participate in console interrupts. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Update comment re: setpgid

3e5246b

Only ctrl-break if separate group; add fallback

33284fe

pjeby force-pushed the fix-3134 branch from 11c221d to 33284fe Compare March 19, 2026 04:13

junegunn approved these changes Mar 20, 2026

View reviewed changes

Only use new kill w/pwsh and unknown/posixy shells

9f5cb66

junegunn requested a review from Copilot March 28, 2026 13:04

Copilot started reviewing on behalf of junegunn March 28, 2026 13:04 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

Comment thread src/util/util_windows.go Outdated

nil check and masking

a89f7ca

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

junegunn reviewed Mar 29, 2026

View reviewed changes

Comment thread src/util/util_windows.go Outdated

Consolidate shell cases

4ecea6f

Co-authored-by: Junegunn Choi <junegunn.c@gmail.com>

junegunn approved these changes Mar 29, 2026

View reviewed changes

junegunn merged commit fff7eda into junegunn:master Mar 29, 2026
6 checks passed

coderabbitai Bot mentioned this pull request Jul 3, 2026

fix(ui_select): add native preview for fzf-native/fzf-tmux ibhagwan/fzf-lua#2753

Merged

Uh oh!

Conversation

pjeby commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

pjeby Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

junegunn Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pjeby Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

junegunn Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

pjeby Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

junegunn Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

pjeby Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

junegunn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pjeby commented Mar 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

junegunn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

junegunn left a comment •

edited

Loading