piping `echo` into another processes creates a job group with negative number

[kaddkaka]

I tried this with sh -c 'env HOME=$(mktemp -d) fish'

> fish --version
fish, version 3.6.1`

> uname -a
Linux Tugge 5.15.0-67-generic #74-Ubuntu SMP Wed Feb 22 14:14:39 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

> echo $TERM
xterm-256color

> wezterm --version
wezterm 20230326-111934-3666303c

> vim --version
VIM - Vi IMproved 8.2

Scenario:

> echo "" | vim -

CTRL-Z

Vim: Reading from stdin...
fish: Job 1, 'echo "" | vim -' has stopped

> fg
fg: There are no suitable jobs

[2]> fg %1
fg: Can't put job 132830, 'echo "" | vim -' to foreground because it is not under job control

[2]> jobs
Job     Group   CPU     State   Command
1       -2      0%      stopped echo "" | vim -

> exit
There are still jobs active:

   PID  Command
     0  echo "" | vim -

A second attempt to exit will terminate them.
Use 'disown PID' to remove jobs from the list without terminating them.

After start echo "" | vim - doing ctrl-z, the jobs list has one item but I'm not able to foreground it because it is "not under job control". Is that expected behavior?

In bash it is possible to foreground the vim process again. I expected the same in fish.

[kaddkaka]

The job group -2 looks fishy. If I start vim in other ways it's possible to foreground again, for example:

• vim
• cat test.txt | vim -

> jobs
Job     Group   CPU     State   Command
3       135441  0%      stopped vim
2       135316  0%      stopped cat test.txt | vim -
1       -2      0%      stopped echo "test" | vim -

[kaddkaka]

echo "" | sleep 10 followed by CTRL-Z:

3       -2      0%      stopped echo "" | sleep 10
2       139297  0%      stopped sleep 1 | echo "" | vim -
1       139182  0%      stopped sleep 1 | sleep 1 | vim -

[zanchey]

-2 isINVALID_PID, so there's a problem with the job construction here. Still happens in master.

[ridiculousfish]

When the first process is a builtin (in this case echo) or fish function, fish does not create a separate process group for the job; instead all processes live in fish's process group.

This has been an issue since fish 1.x; for example if you do this:

 function sleeper; while true; sleep 10; echo "zzz"; end; end
 sleeper | true

then control-Z will send the sleep to the background, and the function just continues on in the loop, spawning more sleeps. It's an issue with no easy solution for shells which do not fork copies of themselves.

Still we should at least fix the output of jobs, the -2 is pretty weird.

[mqudsi]

Taking a step back, two obvious solutions:

• conditionally fork only when we have to for cases like this (directly into the builtin then exiting immediately after), or
• if we really want to avoid that, we could additionally compile each builtin to its own binary (or all of them to a single binary, hard-linked to the different builtin names the same way busybox does) and run that binary directly - but this means making sure builtins no longer used shared state, which is no small undertaking

cons are that builtin performance when used in a pipeline like this will suffer, which is probably acceptable.