New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subprocess crashes not reported within blocks #4962

Closed
mqudsi opened this Issue May 7, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@mqudsi
Copy link
Contributor

mqudsi commented May 7, 2018

~> foo
fish: “foo…” terminated by signal SIGABRT (Abort)
~> begin; foo; end;
~>

Gave me a hell of a time trying to understand why my script was failing without any visible errors.

@zanchey zanchey added the bug label May 8, 2018

@zanchey zanchey added this to the fish-future milestone May 8, 2018

@faho

This comment has been minimized.

Copy link
Member

faho commented May 8, 2018

This seems to have been originally added on purpose. See parse_execution.cpp:run_1_job():

    job->set_flag(JOB_SKIP_NOTIFICATION,
                  is_subshell || is_block || is_event || !shell_is_interactive());

It seems safe to remove blocks from this, though I'd probably even report crashes (SIGABRT) for events and "subshell"s (this is talking about command substitutions).

@faho

This comment has been minimized.

Copy link
Member

faho commented May 8, 2018

Yeah, this was added way back in 2005, in 838ba08.

With the full commit message of "Event handling and job reaping updates".

@faho

This comment has been minimized.

Copy link
Member

faho commented May 8, 2018

Bash's policy here seems to be to notify about crashes except in command substitutions - even if the shell isn't interactive!

So that would be

    job->set_flag(JOB_SKIP_NOTIFICATION,
                  is_subshell);

Which I don't quite get.

Anyone got any ideas for a better policy than what we have now?

@faho

This comment has been minimized.

Copy link
Member

faho commented Sep 18, 2018

So.... how about we just always report crashes?

I'm not entirely sure how to define "crash", though. My best attempt is "everything that was killed via one of the following signals: SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGSYS".

@mqudsi

This comment has been minimized.

Copy link
Contributor

mqudsi commented Sep 18, 2018

I would be on board with that.

@faho faho closed this in 0e82fcd Sep 18, 2018

@faho faho modified the milestones: fish-future, fish-3.0 Sep 18, 2018

@faho

This comment has been minimized.

Copy link
Member

faho commented Sep 18, 2018

Gracias!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment