Preview commands sometimes not being killed

[ErikReider]

Checklist

• I have read through the manual page (man fzf)
• I have searched through the existing issues
• For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.67.0 (Fedora)

OS

• Linux
• macOS
• Windows
• Etc.

Shell

• bash
• zsh
• fish

Problem / Steps to reproduce

Similar to #1384 which still seems to be an issue on my end. I started noticing this when my CPU would be pinned at 100% usage due to multiple less processes still running.

After some digging around, I was able to reproduce this issue by following these steps (took some tries on my laptop):

1. Run echo PATH_TO_A_BINARY_FILE | fzf --preview 'less {}'
2. Wait for around 10s
3. Close fzf with Ctrl+c

It's interesting that when running less PATH_TO_A_BINARY_FILE, less would prompt with "may be a binary file. See it anyway?", but this doesn't lead to my high CPU usage + less still running after fzf exists. I'm not sure what fzf does in the background, but might that be the root cause?

This could easily be fixed on my end by running less with the -f flag, but this is still an edge-case that others might encounter.

Thanks for your time :)

[LangLangBart]

Preview commands sometimes not being killed
CPU would be pinned at 100% usage due to multiple less processes still running

Thx, for reporting it, I have noticed this as well.

---

On Fedora, reproduction is much more reliable than on macOS, happens for me
in the docker environment every time.

1. run podman/docker

# https://hub.docker.com/_/fedora
podman run --quiet --rm --interactive --tty fedora:rawhide sh -ec '
dnf install -qy less fzf top
/bin/bash'

2. When the prompt is present run

fzf --bind 'start:reload:true' --preview 'less /bin/sh' --with-shell 'bash -c'

3. Press Ctrl+C when u see 'Loading ...' in the preview window

4. Run top and observe the CPU intensive less process

[root@a887f18f6bf0 src]# top
    PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
    119 root      20   0    4324   2212   1996 R 100.0   0.1   0:36.97 less
      1 root      20   0    4864   3880   3256 S   0.0   0.2   0:00.01 bash
    165 root      20   0    8436   4824   2740 R   0.0   0.2   0:00.00 top

---

On macOS, it takes a couple of attempts. In the GIF below, I run the
watch command to check running less processes, and in a second terminal I
repeatedly run the same fzf command and press Ctrl+C. Eventually, a less
process is not cleaned up.

# TERMINAL 1
# brew install watch
watch -n1 'ps -o pid,ppid,pcpu,state,command -p $(pgrep -x less) 2>/dev/null'
# TERMINAL 2
fzf --bind 'start:reload:true' --preview 'less /bin/sh' --with-shell 'bash -c'

@junegunn can u reproduce as well ?

---

Killing the preview first before signalling to quit seems to work for me. @ErikReider can u confirm ?

--- a/src/terminal.go
+++ b/src/terminal.go
@@ -5597,8 +5597,9 @@ func (t *Terminal) Loop() error {
    }
 
-   t.eventBox.Set(EvtQuit, quitSignal{code, nil})
-   t.running.Set(false)
    t.killPreview()
    cancel()
+   time.Sleep(10 * time.Millisecond)
+   t.running.Set(false)
+   t.eventBox.Set(EvtQuit, quitSignal{code, nil})
  }()

[LangLangBart]

The less maintainer addressed the CPU-intensive busy-loop bug - will be fixed in a future release.

• Ref: Infinite loop at 100% CPU when tty unavailable and binary file prompts for input gwsw/less#719

The issue about some preview processes surviving fzf's cleanup is still valid though.

[junegunn]

# TERMINAL 1
# brew install watch
watch -n1 'ps -o pid,ppid,pcpu,state,command -p $(pgrep -x less) 2>/dev/null'
# TERMINAL 2
fzf --bind 'start:reload:true' --preview 'less /bin/sh' --with-shell 'bash -c'

@junegunn can u reproduce as well ?

Thanks, I can reproduce.

Killing the preview first before signalling to quit seems to work for me.

I'd like to find a solution that reliably works without an arbitrary sleep in the code. I'll look into it.

[LangLangBart]

I'd like to find a solution that reliably works without an arbitrary sleep in the code.

Thanks, i hoped you would. Tested it and works well.