Command execution returns success on segfault

[jrk]

Commands silently succeed when the underlying process segfaults. e.g.:

sh.Command('sh')('-c', 'kill -SEGV $$')

returns without error, where:

sh.Command('false')

raises a non-zero exit code, as expected.

Testing from the shell, the segfaulting processes all do set a non-zero exit code (139), but it is silently dropped by sh. (The same is not true of subprocess.check_call.)

(This is on OS X 10.8.1, with the latest sh.py from git.)

[amoffat]

I'll can take a look at this over the weekend. Btw, I'm looking at some of your CG papers, specifically the volumetric ray paper. I've been writing a graphics engine for fun, and I added crepuscular rays as a post-process effect in screen space. It's got some quirks vs "true" volumetric rays, but it's pretty fast. Looking at your paper though, your implementation seems to have good performance as well...

[amoffat]

Hm, I'm seeing different behavior on linux:

import sh

cmd = sh.Command('sh')('-c', 'kill -SEGV $$')
print cmd.exit_code # -11

It makes sense that a progress that ends from a signal does not raise an exception, this was a design decision I made. What doesn't make sense is that you're seeing exit code 139 for the process you're killing. Was 139 for some other process that segfaulted, or can you confirm that you're seeing 139 from the above code too?

[jrk]

In that case, I'm not clear:

Which exit conditions do trigger exceptions, and which don't?

Some certainly do, and these segfaults certainly don't, but the distinction is unclear.

[amoffat]

A process that ends from a signal does not raise an exception, and its return code is -signal_num (this is how subprocess sets the exit code as well). Anything that was not ended by a signal and has a positive exit code, raises.

[amoffat]

So if you could get back to me on this:

Was 139 for some other process that segfaulted, or can you confirm that you're seeing 139 from the above code too?

It would help me debug

[jrk]

That was the exit code I saw in Bash. In Sh, it maps to -11 as you expected.

[amoffat]

Ah ok, so this is expected behavior then. I don't have any plans to change how sh responds to signals that trigger an exit, unless you have a good argument for it. The original reasoning was that if you had a big python script that used sh, and one of your subprocesses was hanging, you could kill the subprocess (or send it some other exit-inducing signal) without causing the script to die (because you probably didn't have a try/except for that signal around it)

[jrk]

That's fine, it was just unexpected behavior. I still suspect the opposite behavior might be more predictable (do the obvious, consistent thing of raising all non-zero exit conditions by default; require try/except or a dedicated setting for the use case you're describing), but for my usage I'm fine with this.

Actually, it might be useful at least to have an option to force exceptions for all unexpected exits, unifying SIGSEGV or SIGKILL with exit(1) etc. Perhaps an option on the Command object or to its call operator.

[amoffat]

Cool, I'll add a note to the documentation about the current behavior and then close this out

[amoffat]

updated docs live

[jordigh]

Can you revisit this decision? I really don't understand the rationale for not raising an exception when a process exits with a signal. This is a worse situation than erroring out with a positive error code. If the process you called segfaulted, well, something is pretty wrong somewhere down the line, so it should be treated with at least the same severity as a nonzero exit code.

Or if you want to treat SIGSEGV and SIGKILL differently, maybe, but I still think that if you kill a long-running process, the whole Python script should also stop.

[amoffat]

We would need to define which signals should throw an exception and which shouldn't. There's quite a few, and it might not be clear on which ones should raise

SIGABRT = 6
SIGALRM = 14
SIGBUS = 7
SIGCHLD = 17
SIGCLD = 17
SIGCONT = 18
SIGFPE = 8
SIGHUP = 1
SIGILL = 4
SIGINT = 2
SIGIO = 29
SIGIOT = 6
SIGKILL = 9
SIGPIPE = 13
SIGPOLL = 29
SIGPROF = 27
SIGPWR = 30
SIGQUIT = 3
SIGRTMAX = 64
SIGRTMIN = 34
SIGSEGV = 11
SIGSTOP = 19
SIGSYS = 31
SIGTERM = 15
SIGTRAP = 5
SIGTSTP = 20
SIGTTIN = 21
SIGTTOU = 22
SIGURG = 23
SIGUSR1 = 10
SIGUSR2 = 12
SIGVTALRM = 26
SIGWINCH = 28
SIGXCPU = 24
SIGXFSZ = 25
SIG_DFL = 0
SIG_IGN = 1

[asharp]

You could always start with a few obvious ones (SIGABRT, SIGALRM, SIGKILL, SIGQUIT, etc), then expand later, depending on what people find.

[jordigh]

Not all of these signals terminate a process. I think any that does terminate a process should raise an exception, i.e. just patch the line

if os.WIFSIGNALED(exit_code): return -os.WTERMSIG(exit_code)

to raise an exception. Sadly, this includes SIGKILL and SIGTERM, but I think it's the right choice. It is, for example, what Perl's IPC::System::Simple does. And whatever else you may think about Perl, they do know their Unix and their signals over there. ;-)

[jordigh]

Thanks, I think this is exactly what was needed.