[Toybox] default SIGPIPE handler that calls _exit(0)?

[Rob Landley]

On 04/25/2015 02:14 PM, enh wrote:
> what's the plan wrt SIGPIPE?

Hadn't made any? I have xexit() doing fflush(0) ||
ferror(stdout) and perror_msg("write") if you redirect
the output to a disk that fills up and such, but
getting killed by a signal has a default behavior
provided by the OS, and I left it alone...

> the desktop is pretty inconsistent.
> many (but not all) commands install a signal handler that
> does _exit(0). others (coreutils 8.21's ls, say) do nothing.
> normally "what you do about SIGPIPE" isn't a problem but on
> Android that leads to a crash report and people filing "ls
> crashed" bugs against me. (our default shell PS setup is
> also noisy about crashes.)

Is exiting due to sigpipe really a crash?

> in the past, like the desktop, we've fixed this on an ad
> hoc basis. i just uploaded a change to toolbox
> (https://android-review.googlesource.com/148331)

How do I get gerrit to show me the darn patch? I clicked
on the change ID and it opened the same "it touches these
two files and we'll show you how many lines were changed
but not _what_ changed" page...

If I right click on "gitiles" next to the commit ID it will
open a tree view of that version, but not a diff view.

Ok, copy the change ID to the clipboard (horrible javascript
thing to turn it into an editing field as soon as I highlighted
it to prevent me from doing a normal cut and paste), then
right click open in new tab the project link in the upper right,
click on a random commit, paste the commit ID over that commit
number... nope, it's the same "what's a diff?" web interface...

Ok, clone the darn git project and git show from the command
line. (Your web interface goes out of its way to be useless.
It puts _effort_ into being useless.)

Ok, so it's basically:

  void handler(int i) {_exit(0);}
  int main(int argc, char *argv[])
  {
    signal(SIGPIPE, handler);

Hmmm... ok, leaving aside your crash detector probably being
overzealous, another problem is toysh. It does some nofork()
commands and if they got a sigpipe the _shell_ would exit.

So my first instinct (puting this in toy_singleinit()) is
inappropriate, and... I guess it really _does_ belong in
main(). Then toysh can set its own signal handlers and they
should stay set. (toy_init() isn't resetting signal
handlers. There's a gazillion subtle things about
toys.recursion that I'll need to audit someday, all of
which really translate to restrictions on who can call
toy_exec(). The man 2 execve page lists the things I need
to worry about.)

Why are we calling _exit() instead of exit()? Does flush(0)
and calling the atexit() chain cause a problem here? (Modulo
it being done from the signal stack which might be a bit
cramped.) There's this whole nofork vague design about
being able to longjmp() from xexit() instead of exit... which
is of course where the above "gazillion subtle things" would
really come in, right now it's not a big deal because you
only really recurse deeply when you "nice nohup taskset setsid
chroot xargs time" and so on, and those generally aren't doing
anything fancy with state _other_ than what they're explicitly
there to manipulate...

> to do this in the
> driver instead. [pretend for a moment that toolbox isn't
> going away...]

It's probably not going away by the Android M deadline, anyway.

> any tool that actually cares about SIGPIPE behavior is
> unaffected because it can install its own SIGPIPE handler.
> any tool that doesn't care is unaffected because it's still
> going to exit; it's just going to exit cleanly.

I'll defer to your definition of "cleanly" for android,
which means I'm adding it with an if (CFG_TOYBOX_ON_ANDROID)
as a bug workaround for what _sounds_ like an overzealous
crash detector. If I'm wrong about that and it should be
enabled generally, I can take the test out later once
people explain what I got wrong.

> does this sound like a good idea for toybox?

It sounds like something you need.

Rob

 1430334548.0