Handler for --on-job-exit $fish_pid runs constantly #7721

faho · 2021-02-16T10:12:39Z

As reported on gitter by @scorphus, another for the "fixed by status job-control full" (#5036) pile:

function on-exit --on-job-exit $fish_pid
    echo on-exit
end

Run this interactively and it will trigger constantly - presumably whenever you run the prompt or similar.

This is presumably because the prompt doesn't have "job-control" enabled and therefore runs with fish's pgroup.

While on-job-exit doesn't make sense here, it shouldn't trigger constantly.

While pid values may be reused, it is logical to assume that fish event handlers coded against a particular job or process id mean just the job that is currently referred to be any given pid/pgrp rather than in perpetuity. This trims the list of registered event handlers nice and early, and as a bonus avoids the issue described in #7721. The cleanup song-and-dance is extremely ugly due to the repeated locking and unlocking of the event handler list. Closes #7221.

mqudsi · 2021-03-06T04:52:04Z

Fixed in e7398c0 but I fat-fingered the commit message and it auto-closed the wrong issue.

scorphus · 2021-03-06T13:56:23Z

Thanks, @mqudsi

We have no idea why this was even a thing. For now simply set it to "all"/"full" (why these two names? no idea) at startup and allow changing it later. Settting it *immediately* when defining the variable sets it too soon because we don't have the interactive signal handlers enabled (including the one for SIGTTOU), so let's first settle for this little piece of awkwardness. This needs widespread testing, so we merge it early, immediately after the release. Fixes #5036 Fixes #5832 Fixes #7721 (and probably numerous others)

When a job is placed in the background, fish will set the `$last_pid` variable. Prior to this change, `$last_pid` was set to the process group leader of the job. However this caussed problems when the job ran in fish's process group, because then fish itself would be the process group leader and commands like `wait` would not work. Switch `$last_pid` to be the actual last pid of the pipeline. This brings it in line with the `$!` variable from zsh and bash. This is technically a breaking change, but it is unlikely to cause problems, because `$last_pid` was already rather broken. Fixes fish-shell#5036 Fixes fish-shell#5832 Fixes fish-shell#7721

When a job is placed in the background, fish will set the `$last_pid` variable. Prior to this change, `$last_pid` was set to the process group leader of the job. However this caussed problems when the job ran in fish's process group, because then fish itself would be the process group leader and commands like `wait` would not work. Switch `$last_pid` to be the actual last pid of the pipeline. This brings it in line with the `$!` variable from zsh and bash. This is technically a breaking change, but it is unlikely to cause problems, because `$last_pid` was already rather broken. Fixes #5036 Fixes #5832 Fixes #7721

faho added the bug Something that's not working as intended label Feb 16, 2021

faho added this to the fish 3.3.0 milestone Feb 16, 2021

mqudsi closed this as completed Mar 6, 2021

faho modified the milestones: fish 3.3.0, fish 3.2.1 Mar 8, 2021

ridiculousfish mentioned this issue May 21, 2021

Fix $last_pid and re-disable non-interactive job control #8021

Closed

github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2021

Handler for --on-job-exit $fish_pid runs constantly #7721

Handler for --on-job-exit $fish_pid runs constantly #7721

faho commented Feb 16, 2021

mqudsi commented Mar 6, 2021

scorphus commented Mar 6, 2021

Handler for --on-job-exit $fish_pid runs constantly #7721

Handler for --on-job-exit $fish_pid runs constantly #7721

Comments

faho commented Feb 16, 2021

mqudsi commented Mar 6, 2021

scorphus commented Mar 6, 2021