Permalink
Browse files

many signal changes

  • Loading branch information...
1 parent 196ce3f commit 1b3c88922e61ad3f2dbd8cb48009b021cc5c4032 @rcaputo committed Jun 3, 2000
Showing with 100 additions and 24 deletions.
  1. +16 −0 Changes
  2. +1 −1 Makefile.PL
  3. +17 −1 README
  4. +56 −20 lib/POE/Kernel.pm
  5. +8 −0 lib/POE/Session.pm
  6. +2 −2 tests/11_signals_poe.t
View
16 Changes
@@ -36,6 +36,22 @@ subversions are available from <http://www.newts.org/~troc/poe.html>.
0.1006 2000.??.?? (!!!)
-----------------------
+Increased the time that t/11_signals_poe.t waits to reap children. On
+memory- and cpu-challenged systems, swap times could delay process
+exits, causing false failures when the test program timed out.
+
+Fimmtiu discovered why it's bad to post _start yourself, and I decided
+it would be best for everyone to document it.
+
+Philip Gwyn patched POE::Kernel to unwind SIGCHLD reaping loops. This
+greatly reduces the amount of work done in POE's stock $SIG{CHLD}
+handlers which in turn reduces the probability that SIGCHLD will
+segfault Perl. Note, however, that the chance of this occurring will
+remain nonzero as long as Perl is not signal safe. (Also, the patch
+wouldn't apply and I sort of redid the code as I spliced the idea into
+the Kernel. It's probably busted, and I should compare it against
+Philip's original patch to make sure it works the way he intended.)
+
Tweaked postbacks. Older Perl versions were not doing the right
things with \@_ in POE::Session's postback() method. Replaced it
with [ @_ ] instead.
View
@@ -11,7 +11,7 @@ WriteMakefile
AUTHOR => 'Rocco Caputo <troc+poe@netrus.net>',
ABSTRACT => 'Event driven state machines and I/O abstractions.',
VERSION_FROM => 'POE.pm',
- dist => { 'COMPRESS' => 'gzip -9',
+ dist => { 'COMPRESS' => 'gzip -9f',
'SUFFIX' => 'gz',
},
PREREQ_PM => { Carp => 0,
View
18 README
@@ -66,7 +66,23 @@ everything. To further this effort, the author wrote a test coverage
reporting program; then he discovered Devel::Coverage. Oh well!
Anyway, here's the test coverage summary for this version:
-... don't forget to put them here ...
+ Source File = Ran / Total = Covered
+ POE.pm = 19 / 19 = 100.00%
+ POE/Component/Server/TCP.pm = 23 / 23 = 100.00%
+ POE/Driver/SysRW.pm = 36 / 54 = 66.67%
+ POE/Filter/HTTPD.pm = 11 / 85 = 12.94%
+ POE/Filter/Line.pm = 15 / 20 = 75.00%
+ POE/Filter/Reference.pm = 4 / 66 = 6.06%
+ POE/Filter/Stream.pm = 2 / 11 = 18.18%
+ POE/Kernel.pm = 616 / 908 = 67.84%
+ POE/Preprocessor.pm = 119 / 134 = 88.81%
+ POE/Session.pm = 88 / 194 = 45.36%
+ POE/Wheel/FollowTail.pm = 5 / 65 = 7.69%
+ POE/Wheel/ListenAccept.pm = 5 / 43 = 11.63%
+ POE/Wheel/ReadWrite.pm = 99 / 183 = 54.10%
+ POE/Wheel/SocketFactory.pm = 187 / 267 = 70.04%
+ All Told = 1229 / 2072 = 59.31%
+
Good luck, and thank you for reading!
View
@@ -528,17 +528,23 @@ sub _poe_signal_handler_pipe {
}
# SIGCH?LD are normalized to SIGCHLD and include the child process'
-# PID and return code.
+# PID and return code. Philip Gwyn rewrote most of the SIGCH?LD code
+# for version 0.1006; it got rewritten again while the patches were
+# manually applied. I expect it to be rewritten a few more times to
+# fix Philip's code back the way it ought to be.
sub _poe_signal_handler_child {
if (defined $_[0]) {
- # Reap until there are no more children.
- while ( ( my $pid = waitpid(-1, WNOHANG) ) > 0 ) {
- {% post_child_signal $poe_kernel, $pid, $? %}
- }
-
- $SIG{$_[0]} = \&_poe_signal_handler_child;
+ # The default SIGCH?LD action is "discard". We set it here to
+ # prevent Perl from catching more SIGCHLD signals while the Kernel
+ # polls for child processes.
+ $SIG{$_[0]} = 'DEFAULT';
+ $poe_kernel->_enqueue_state( $poe_kernel, $poe_kernel,
+ EN_SCPOLL, ET_SCPOLL,
+ [ ],
+ time(), __FILE__, __LINE__
+ );
}
else {
warn "POE::Kernel::_signal_handler_child detected an undefined signal";
@@ -1289,7 +1295,6 @@ sub run {
if ($timeout || keys(%$kr_handles)) {
# Check filehandles, or wait for a period of time to elapse.
-
my $hits = select( my $rout = $kr_vectors->[VEC_RD],
my $wout = $kr_vectors->[VEC_WR],
my $eout = $kr_vectors->[VEC_EX],
@@ -1651,11 +1656,15 @@ sub _invoke_state {
# Non-blocking wait for a child process. If one was reaped,
# dispatch a SIGCHLD to the session who called fork.
- while ( ( my $pid = waitpid(-1, WNOHANG) ) > 0 ) {
+ my $pid = waitpid(-1, WNOHANG);
+
+ # A child stopped, or something.
+
+ if ($pid > 0) {
# Determine if the child process is really exiting and not just
# stopping for some other reason. This is perl Perl Cookbook
- # recipe 16.19.
+ # recipe 16.19 and the waitpid(2) manpage.
if (WIFEXITED($?)) {
@@ -1669,28 +1678,55 @@ sub _invoke_state {
exists $self->[KR_SESSIONS]->{$parent_session}
);
- # Enqueue the signal.
+ # Enqueue the signal event.
$self->_enqueue_state( $parent_session, $self,
EN_SIGNAL, ET_SIGNAL,
[ 'CHLD', $pid, $? ],
time(), __FILE__, __LINE__
);
}
+
+ # Enqueue an immediate subsequent wait in case another child
+ # process is waiting.
+
+ $self->_enqueue_state( $poe_kernel, $poe_kernel,
+ EN_SCPOLL, ET_SCPOLL,
+ [ ],
+ time(), __FILE__, __LINE__
+ );
+
+ }
+
+ # An error occurred.
+
+ elsif ($pid == -1) {
+
+ # waitpid(2) was interrupted. Retry immediately.
+
+ if ($! == EINTR) {
+ $self->_enqueue_state( $poe_kernel, $poe_kernel,
+ EN_SCPOLL, ET_SCPOLL,
+ [ ],
+ time(), __FILE__, __LINE__
+ );
+ }
+
+ # Some other error occurred. Assume we're stopping the wait
+ # loop. Warn if it's something unexpected.
+
else {
- last;
+ $SIG{CHLD} = \&_poe_signal_handler_child if exists $SIG{CHLD};
+ $SIG{CLD} = \&_poe_signal_handler_child if exists $SIG{CLD};
+ warn $! if $! and $! != ECHILD;
}
}
- # If there still are processes waiting, post another EN_SCPOLL for
- # later.
+ # Nothing is left to wait for. Stop the wait loop.
- if (scalar keys %{$self->[KR_PROCESSES]}) {
- $self->_enqueue_state( $self, $self,
- EN_SCPOLL, ET_SCPOLL,
- [],
- time() + 1, __FILE__, __LINE__
- );
+ else {
+ $SIG{CHLD} = \&_poe_signal_handler_child if exists $SIG{CHLD};
+ $SIG{CLD} = \&_poe_signal_handler_child if exists $SIG{CLD};
}
}
View
@@ -1119,6 +1119,14 @@ convention. It's therefore recommended not to use a single leading
underscore in custom state names, since there's a small but positive
probability of colliding with future standard events.
+Predefined states generally have serious side effects. The _start
+state, for example, performs much of the task of setting up a session.
+Posting a redundant _start state transition will dutifully attempt to
+allocate a session that already exists, which will in turn do
+terrible, horrible things to the Kernel's internal data. Such things
+would normally be outlawed outright, but the extra overhead to check
+for them hasn't yet been deemed worthwhile. Please be careful!
+
Here now are the predefined standard states, why they're invoked, and
what their parameters mean.
View
@@ -59,7 +59,7 @@ POE::Session->create
print "not ok 1 # forked $_[HEAP]->{forked} out of $fork_count\n";
}
- $_[KERNEL]->delay( time_is_up => 60 );
+ $_[KERNEL]->delay( time_is_up => 120 );
},
_stop =>
@@ -76,7 +76,7 @@ POE::Session->create
catch_sigchld =>
sub {
$_[HEAP]->{reaped}++;
- $_[KERNEL]->delay( time_is_up => 5 );
+ $_[KERNEL]->delay( time_is_up => 15 );
},
time_is_up =>

0 comments on commit 1b3c889

Please sign in to comment.