autodie icon indicating copy to clipboard operation
autodie copied to clipboard

kill 0, $pid should probably not throw exception

Open jarich opened this issue 10 years ago • 11 comments

kill 0, $pid is an effective way to determine whether a process is still running and can accept signals from this process (without actually killing it). If the process exists, this returns true, and if the process does not, it would normally return false.... unless autodie is in use in which case it throws an exception and kills the program.

jarich avatar Apr 08 '14 01:04 jarich

From perldoc -f kill

"If SIGNAL is either the number 0 or the string ZERO (or SIGZZERO ), no signal is sent to the process, but kill checks whether it's possible to send a signal to it (that means, to be brief, that the process is owned by the same user, or we are the super-user). This is useful to check that a child process is still alive (even if only as a zombie) and hasn't changed its UID. See perlport for notes on the portability of this construct."

http://perldoc.perl.org/functions/kill.html

jarich avatar Apr 08 '14 01:04 jarich

Oh drat. I think you're right, that kill 0, $pid shouldn't be autodying, and we already have precedence here in the behaviour of non-blocking flock calls.

I'm teaching right now (as you know!), but as a workaround you can always use CORE::kill(0, $pid), but I know that you already know that.

I need to think about whether changing this may break existing code, and if so should we do so. I may even ask @rjbs for spiritual guidance.

~ P

pjf avatar Apr 08 '14 03:04 pjf

Sorry for my late reply!

My first thought is: if you're using kill 0, you're doing it for the return value. I'd want kill to be fatal on in void context. If you're using kill 0 in void context, you are probably confused. I realize that autodie is not big on "...in void context," however. (Perl 6 has a nice solution for this… :-/)

I think it could break existing code. My gut tells me that it's unlikely. I think you could get away with this change. On the other hand, it's a complication to the nearly universal rule that "with autodie, affected routines go from 'return false' to 'die' on failure," which doesn't thrill me.

Finally and maybe most importantly: I was unable to quickly find the implementation of autodie or Fatal's kill. I worry that they are only throwing an exception when kill would return false, which is not necessarily the right condition. If you try to signal 4 processes, but only 3 can be sent to, is that a fatal error? Should it be?

rjbs avatar Apr 20 '14 02:04 rjbs

@rjbs: The reason why you probably failed to find the implementation of kill is that it is auto-generated. It is listed in %Returns_num_things_changed, so it fails if "$retval != (@_ - X)" [where X is the value listed in %Returns_num_things_changed].

nthykier avatar Sep 22 '14 18:09 nthykier

Closed with pull request #72

nthykier avatar Mar 03 '16 21:03 nthykier

Still happens. Neither "there" nor "done" appears in this program:

njh@microcenter:/tmp$ cat autodie #!/usr/bin/env perl

use strict; use warnings; use autodie qw(:all);

if(kill(0, 10000)) { print "there\n"; }

njh@microcenter:/tmp$ ./autodie Can't kill('0', '10000'): No such process at ./autodie line 7 njh@microcenter:/tmp$

nigelhorne avatar Jun 05 '19 16:06 nigelhorne

It appears the code at https://github.com/pjf/autodie/blob/master/lib/Fatal.pm#L1043 still expects to autodie when in a non-void context. kill(0, ...) should never die is what I think we're saying?

toddr avatar Dec 24 '19 00:12 toddr

Yes, that is what I'm saying.

nigelhorne avatar Dec 26 '19 14:12 nigelhorne

I could see a use case for the status quo, i.e., “die if $pid is gone”. Also, this interface has worked this way for a while, right?

Were autodie.pm new, it would totally make sense to make this not throw, but given that the ship has (long since) sailed, IMO there’s not a compelling case for what is, strictly speaking, a breaking change.

That aside, if this change does go through, please ensure that kill('ZERO', $pid) is treated the same way.

FGasper avatar Dec 30 '19 16:12 FGasper

Maybe we need a docs fix with an example explaining how to do kill 0?

toddr avatar Dec 30 '19 16:12 toddr

Better, IMO, to document this in the POD as one of a list of “potential gotchas” than to change it.

FGasper avatar Dec 30 '19 16:12 FGasper