| Subject: | Signals must not touch the queue |
There is a race condition in POE::Queue::Array :
If a signal handler modifies the queue while the main program is also
modifying it, all offsets used by ->_find_item (or others) are in error.
The included patch is a minimalist work around: signal handlers put
events into a shadow queue, which gets copied to the main queue when
expedient.
Another work around is to use POE::XS::Queue::Array.
Another idea would be to borrow a leaf from erlang. Erlang has a
shared-nothing threading model. If we conceive of the signal handlers
running in one thread, and main loop running in another thread, we need
to make sure they can't stomp on each other. The included patch does
this by copying the shadow queue inside of a grep, which should be
signal-safe.
Erlang, however, would use a pipe or socket to send messages between the
threads. We could use POE::Pipe::OneWay to send very short messages
from the signal handlers to the main loop. A simple string like 'CHLD'
or 'PIPE', or even just the signal's number. This would wake up the
main loop from its select (or poll or whatever). It would then enqueue
the real events in the main queue.
If done this way, we need to rewrite _test_if_kernel_is_idle so that it
ignores the signal-pipe, but only if it is empty.
BTW, this is only an issue for PerlSignals. For the POE::Loops that
implement loop_watch_signal() (Event_Lib, EV), it shouldn't be an issue.
What's more, doing the above would remove the signal polling that
happens in POE::Loop::TkActiveState, for instance.
| Subject: | Philip_Gwyn-POE-signal-shadow-queue.01.patch |
Index: lib/POE/Resource/Signals.pm
===================================================================
--- lib/POE/Resource/Signals.pm (revision 2571)
+++ lib/POE/Resource/Signals.pm (working copy)
@@ -467,7 +467,7 @@
return if $polling_for_signals;
$polling_for_signals = 1;
- $self->_data_ev_enqueue(
+ $self->_data_shadow_enqueue(
$self, $self, EN_SCPOLL, ET_SCPOLL, [ ],
__FILE__, __LINE__, undef, time(),
);
@@ -587,6 +587,9 @@
# If waitpid() returned 0, then we have child processes.
$kr_child_procs = !$pid;
+ if (TRACE_SIGNALS) {
+ _warn("$$: <sg> kr_child_procs=$kr_child_procs" );
+ }
unless ( USE_SIGCHLD ) {
@@ -611,9 +614,46 @@
sub _data_sig_child_procs {
return if !USE_SIGCHLD and !$polling_for_signals;
+ if (TRACE_SIGNALS) {
+ _warn("$$: <sg> _data_sig_child_procs kr_child_procs=$kr_child_procs" );
+ }
return $kr_child_procs;
}
+# This is the shadow queue, used by the signal handlers.
+# Modifying the kernel's queue from a signal handler is not safe.
+# So any new events go into this queue, and get copied to the main queue
+# by _data_sig_enqueue().
+my @shadow_queue;
+
+sub _data_shadow_is_empty {
+ return 0==@shadow_queue;
+}
+
+sub _data_shadow_enqueue {
+ my $self = shift;
+ if (TRACE_SIGNALS) {
+ _warn("$$: <sg> shadow queue signal = ", $_[2],
+ ( 0==@{$_[4]} ? '' : " SIG$_[4][0]" )
+ );
+ }
+ push @shadow_queue, [ @_ ];
+}
+
+sub _data_shadow_requeue {
+ my( $self ) = @_;
+ return unless @shadow_queue;
+ my @temp;
+ # I HOPE this is signal-safe and atomic. If a bit excessive.
+ @shadow_queue = grep { push @temp, $_; 0 } @shadow_queue;
+ if (TRACE_SIGNALS) {
+ _warn("$$: <sg> shadow queue -> main queue ", (0+@temp), " signals." );
+ }
+ foreach my $todo ( @temp ) {
+ $self->_data_ev_enqueue( @$todo );
+ }
+}
+
1;
__END__
Index: lib/POE/Kernel.pm
===================================================================
--- lib/POE/Kernel.pm (revision 2571)
+++ lib/POE/Kernel.pm (working copy)
@@ -635,6 +635,7 @@
$kr_queue->get_item_count() > $idle_queue_size or
$self->_data_handle_count() or
$self->_data_extref_count() or
+ $self->_data_shadow_is_idle() or
$self->_data_sig_child_procs()
) {
$self->_data_ev_enqueue(
Index: lib/POE/Loop/Gtk.pm
===================================================================
--- lib/POE/Loop/Gtk.pm (revision 2571)
+++ lib/POE/Loop/Gtk.pm (working copy)
@@ -241,6 +241,7 @@
$self->_data_stat_add('idle_seconds', time() - $last_time);
}
+ $self->_data_shadow_requeue();
$self->_data_ev_dispatch_due();
$self->_test_if_kernel_is_idle();
Index: lib/POE/Loop/TkCommon.pm
===================================================================
--- lib/POE/Loop/TkCommon.pm (revision 2571)
+++ lib/POE/Loop/TkCommon.pm (working copy)
@@ -132,6 +132,7 @@
sub loop_do_timeslice {
my $self = shift;
+ $self->_data_shadow_requeue();
# Check for a hung kernel.
$self->_test_if_kernel_is_idle();
my $now;
Index: lib/POE/Loop/IO_Poll.pm
===================================================================
--- lib/POE/Loop/IO_Poll.pm (revision 2571)
+++ lib/POE/Loop/IO_Poll.pm (working copy)
@@ -221,6 +221,7 @@
sub loop_do_timeslice {
my $self = shift;
+ $self->_data_shadow_requeue();
# Check for a hung kernel.
$self->_test_if_kernel_is_idle();
Index: lib/POE/Loop/Tk.pm
===================================================================
--- lib/POE/Loop/Tk.pm (revision 2571)
+++ lib/POE/Loop/Tk.pm (working copy)
@@ -197,6 +197,7 @@
# Tk filehandle callback to dispatch selects.
sub _loop_select_callback {
my ($fileno, $mode) = @_;
+ $poe_kernel->_data_shadow_requeue();
$poe_kernel->_data_handle_enqueue_ready($mode, $fileno);
$poe_kernel->_test_if_kernel_is_idle();
}
Index: lib/POE/Loop/Select.pm
===================================================================
--- lib/POE/Loop/Select.pm (revision 2571)
+++ lib/POE/Loop/Select.pm (working copy)
@@ -143,6 +143,7 @@
sub loop_do_timeslice {
my $self = shift;
+ $self->_data_shadow_requeue();
# Check for a hung kernel.
$self->_test_if_kernel_is_idle();
Index: lib/POE/Loop/Event.pm
===================================================================
--- lib/POE/Loop/Event.pm (revision 2571)
+++ lib/POE/Loop/Event.pm (working copy)
@@ -162,6 +162,7 @@
$self->_data_stat_add('idle_seconds', time() - $last_time);
}
+ $self->_data_shadow_requeue();
$self->_data_ev_dispatch_due();
$self->_test_if_kernel_is_idle();
@@ -191,6 +192,7 @@
);
$self->_data_handle_enqueue_ready($mode, $fileno);
+ $self->_data_shadow_requeue();
$self->_test_if_kernel_is_idle();
}
Index: lib/POE/Loop/PerlSignals.pm
===================================================================
--- lib/POE/Loop/PerlSignals.pm (revision 2571)
+++ lib/POE/Loop/PerlSignals.pm (working copy)
@@ -30,7 +30,7 @@
POE::Kernel::_warn "<sg> Enqueuing generic SIG$_[0] event";
}
- $poe_kernel->_data_ev_enqueue(
+ $poe_kernel->_data_shadow_enqueue(
$poe_kernel, $poe_kernel, EN_SIGNAL, ET_SIGNAL, [ $_[0] ],
__FILE__, __LINE__, undef, time()
);
@@ -42,7 +42,7 @@
POE::Kernel::_warn "<sg> Enqueuing PIPE-like SIG$_[0] event";
}
- $poe_kernel->_data_ev_enqueue(
+ $poe_kernel->_data_shadow_enqueue(
$poe_kernel, $poe_kernel, EN_SIGNAL, ET_SIGNAL, [ $_[0] ],
__FILE__, __LINE__, undef, time()
);