Crashes in prompt cause a repaint loop

[faho]

fish_job_summary calls commandline -f repaint, which, if it is called via something in fish_prompt, causes it to loop.

I found this when I noticed that fossil crashes if $PWD doesn't exist, so the test case is:

function fish_prompt; fossil branch current; end
mkdir foo
cd foo
rm -rf $PWD

It'll end up printing a constant stream of

fish: Job 1, 'fossil branch current' terminated by signal SIGABRT (Abort)

until you press ctrl-c. While this should be a bug in fossil, the general principle stands.

I'm not sure how to detect this and turn fish_job_summary off.

[lavafroth]

Hi. I have been looking into this but I can't seem to reproduce the bug. Perhaps this has been fixed?

[faho]

Yup, the repaint was incidentally removed in 9158395

[krobelus]

The repaint is still here, in the poorly-named __fish_echo. I can reproduce with

#include <stdlib.h>
int main() { abort(); }

It's not super clear what we should do here. A fish_job_summary during exec_prompt can be valid, for commands started before fish_prompt.
I guess we can try to set a limit on the number of consecutive repaints triggered by things like fish_job_summary. Or fall back to a default prompt once we notice it crashing

[faho]

Ah, turns out testing with SIGKILL doesn't work.

I had used bash -c 'kill -9 $$', which doesn't trigger fish_job_summary. It does with kill -6.

[ridiculousfish]

The looping is because of this:

    fn prepare_to_select(&mut self) {
        // Fire any pending events and reap stray processes, including printing exit status messages.
        event::fire_delayed(self.parser);
        if job_reap(self.parser, true) {
            reader_schedule_prompt_repaint();
        }
    }

that is, right before we wait for user input, we reap processes and allow printing messages; if a message is printed we schedule a prompt repaint and then react to that instead of select:

            // We are going to block; but first allow any override to inject events.
            self.prepare_to_select();
            if let Some(mevt) = self.try_pop() {
                return Some(mevt);
            }

still unclear what the right approach is. It's worth noting that this issue seems to have existed forever; still if we can fix it we should since it's a very bad state to get into.

[krobelus]

Ah so we never even read stdin.
We could defer reaping jobs from prompt generation X until the current prompt generation is X+1.
This change need only apply to jobs spawned within exec_prompt.
That should slow down the loop to only move one cycle whenever there is a legit repaint.

I haven't explored all alternatives, one of them would be an interrupt key that we always prioritize over all other events (possibly such keys could replace some signal-handling logic)