So, I've checked this a bit, and this is the issue as I understand it:

For certain cases, fish creates a "keepalive" process. This is used to prevent reuse of the foreground pgroup leader PID, which would grant that new process access to the terminal. (Which in itself is an edgecase's edgecase, but theoretically more sound, so okay)

However, the check for a keepalive process is this:

    if (j->get_flag(JOB_CONTROL) && !exec_error) {
        for (const process_ptr_t &p : j->processes) {
            if (p->type != EXTERNAL && (!p->is_last_in_job || !p->is_first_in_job)) {
                needs_keepalive = true;
                break;
            }
        }
    }

So, if the job contains any process that isn't "EXTERNAL" (e.g. a builtin) and that process isn't both the first and last in the job (which would itself mean the job has only one process), create a keepalive process.

This keepalive process then becomes the foreground pgroup leader via set_child_group(j, keepalive.pid).

But in this case - read is executed in the fish process, so the pgroup that reads isn't the foreground pgroup. So it gets SIGTTIN/EIO/whatever, and that presents as this issue.

As I understand it, the process that should be allowed to read() from the terminal here is the first in the pipe - the read. Which is fish itself. So we don't actually need set_child_group, and we don't actually need the keepalive process (since fish itself survives).

And the process that should be allowed to write() to the terminal should be the cat. However IIUC the cat is actually started later since we buffer the read output (I can't actually see a running cat process here), so we don't need to explicitly do anything more.

TBD is the actual conditions that a keepalive process is necessary.

And now I'm down the job control rabbit hole again...

Uh oh!

read | cat crashes fish #4540

Description

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions

`read | cat` crashes fish #4540