Home > Archive > PERL POE > November 2005 > Nastiness, and wrapping up signal reforms
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
| Author |
Nastiness, and wrapping up signal reforms
|
|
| Jonathan Steinert 2005-10-19, 7:05 pm |
| Quickly, my ideas on how things should be are as follows:
1) $SIG{CHLD} should never be set to "IGNORE" under POE.
2) Whenever a process is forked (with a few exceptions [1]) the code
MUST assert an interest in process endings via Kernel->sig(CHLD).
3) Signal handlers must hold sessions alive when registered.
4) Rocco says he would probably like to see mandatory warnings if POE
cleans up any processes when nobody is looking; when this occurs is
still a decision to be made in my mind.
[1] Exceptions include anything that forks but performs its own waitpid.
system(), qx//, etc.
What I would like feedback on is what these ideas will break on people
and why? Do people think these are the right answers? Any better ideas?
Any questions or comments?
Expanding on these ideas:
1)
When $SIG{CHLD} is set to "IGNORE" all calls to wait() or waitpid() will
result in -1. This breaks the expected use of those calls, as well as
system() and friends as well.
The alternative solution to this problem is to override the builtin
system() with something that looks like
sub system {
my $handler = $SIG{CHLD};
local $SIG{CHLD} = "DEFAULT";
my $return = CORE::system(@_);
ref($handler) eq 'CODE') and $handler->();
return $return;
}
This of course /will/ need to be done if we ever use a real signal
handler for CHLD, and will probably need to be done in XS to get the
magic of system {$foo} (@bar) to work.
2)
With the '_signal' event gone, all signals are subscription based, and
we no longer need to run the CHLD poll loop all the time. This means
that zombies could be created if you don't assert an interest (currently
via sig(CHLD => 'event'))
If for some reason you want zombies, too bad. There is no way to
guarantee that they will be created.
3)
When a session asserts interest in CHLD, we need to hold the session
alive till it is done listening. This logic should extend to any signal
as they are a form of IPC. Currently in POE signal watching does not
hold sessions alive, so the problem with this is what this change will
break.
4)
The problem is deciding when to issue these warnings. Should the reaper
loop run all the time and issue warnings when nobody is listening for a
few versions to help migrate people, or perhaps do this instead during
kernel shutdown? I personally will opt for the former.
--Jonathan Steinert
| |
| Rocco Caputo 2005-10-19, 7:05 pm |
| On Oct 19, 2005, at 14:08, Jonathan Steinert wrote:
> Quickly, my ideas on how things should be are as follows:
> 1) $SIG{CHLD} should never be set to "IGNORE" under POE.
s/under/by/; # The user can do what they want, 'though we don't need
to support it.
> 2) Whenever a process is forked (with a few exceptions [1]) the code
> MUST assert an interest in process endings via Kernel->sig(CHLD).
The code asserting interest needn't be the same code that forks the
process.
> 3) Signal handlers must hold sessions alive when registered.
Big change. I don't mind this; the old semantics of not holding a
reference count were tied to _signal, which delivered signals without
sessions explicitly asking for them. _signal is gone now, so we can
tie the explicit interest of sig() into a reference count to keep the
session alive.
> 4) Rocco says he would probably like to see mandatory warnings if POE
> cleans up any processes when nobody is looking; when this occurs is
> still a decision to be made in my mind.
Setting $SIG{CHLD} = "IGNORE" was a direct response to people who
complained that POE "created zombies". Letting Perl reap child
processes is a generally bad thing. It introduces race conditions
and eats signals that system() needs, for two examples.
Setting the handler back to "DEFAULT" will stop the races but bring
back the zombies. Users who don't want zombies should set up a
SIGCHLD handler. A mandatory signal handler will educate developers
and users, which is probably what I should have done in the first place.
> [1] Exceptions include anything that forks but performs its own
> waitpid.
> system(), qx//, etc.
>
> What I would like feedback on is what these ideas will break on people
> and why? Do people think these are the right answers? Any better
> ideas?
> Any questions or comments?
No ideas here. I'm prepared to fix anything of mine that this
breaks. A mandatory warning will be a great way to point out my
errors. I vote FOR.
> Expanding on these ideas:
>
> 1)
> When $SIG{CHLD} is set to "IGNORE" all calls to wait() or waitpid()
> will
> result in -1. This breaks the expected use of those calls, as well as
> system() and friends as well.
>
> The alternative solution to this problem is to override the builtin
> system() with something that looks like
>
> sub system {
> my $handler = $SIG{CHLD};
> local $SIG{CHLD} = "DEFAULT";
> my $return = CORE::system(@_);
> ref($handler) eq 'CODE') and $handler->();
> return $return;
> }
>
> This of course /will/ need to be done if we ever use a real signal
> handler for CHLD, and will probably need to be done in XS to get the
> magic of system {$foo} (@bar) to work.
Blah. I'd rather have POE do the right thing than patch around it
doing the wrong thing.
> 2)
> With the '_signal' event gone, all signals are subscription based, and
> we no longer need to run the CHLD poll loop all the time. This means
> that zombies could be created if you don't assert an interest
> (currently
> via sig(CHLD => 'event'))
>
> If for some reason you want zombies, too bad. There is no way to
> guarantee that they will be created.
Agreed.
> 3)
> When a session asserts interest in CHLD, we need to hold the session
> alive till it is done listening. This logic should extend to any
> signal
> as they are a form of IPC. Currently in POE signal watching does not
> hold sessions alive, so the problem with this is what this change will
> break.
An alternative would be to assert interest in a particular child
process, based on child PID. This would be a one-shot signal
request, and it would expire when the child process is reaped.
Unfortunately sig()'s syntax isn't flexible enough to support this
change.
Come to think of it, though, this would solve about half the
headaches people have when catching SIGCHLD. Maybe a special sigchld
(event => @extra_args) can be added once the other headaches are
worked out.
> 4)
> The problem is deciding when to issue these warnings. Should the
> reaper
> loop run all the time and issue warnings when nobody is listening
> for a
> few versions to help migrate people, or perhaps do this instead during
> kernel shutdown? I personally will opt for the former.
I prefer the latter. While it means that a process will leak zombies
at runtime, the reason for the leakage should become apparent when
the program's stopped. Screwing up like this should be at least a
little painful.
--
Rocco Caputo - rcaputo@pobox.com
| |
| Rocco Caputo 2005-11-07, 7:48 pm |
| On Oct 19, 2005, at 14:08, Jonathan Steinert wrote:
> Quickly, my ideas on how things should be are as follows:
Looks like there are no objections. I say go for it.
--
Rocco Caputo - rcaputo@pobox.com
|
|
|
|
|