Skip to content

Commit

Permalink
Clear out the event queue by session ID rather than reference.
Browse files Browse the repository at this point in the history
I suspect this was causing defunct parent-process events to linger in
the child-process queue after POE::Kernel->stop() was called.  Their
dispatch in the child process may have caused the "hilarity" I have
seen in the wild.
  • Loading branch information
rcaputo committed Mar 25, 2011
1 parent 32f642c commit 9654581
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 28 deletions.
6 changes: 6 additions & 0 deletions lib/POE.pm
Expand Up @@ -636,6 +636,12 @@ Filters. He greatly improved L<POE::Wheel::FollowTail|POE::Wheel::FollowTail>,
contributions include the basic Block filter, as well as Stackable, contributions include the basic Block filter, as well as Stackable,
RecordBlock, Grep and Map. RecordBlock, Grep and Map.
=item Plixer International
Plixer International is at L<http://plixer.com/>. Their sponsorship
has helped POE 1.300 and beyond be significantly more robust using
iThreads, especially when using fork() in Windows.
=item Robert Seifer =item Robert Seifer
Robert Seifer is <e-mail unknown>. He rotates IRC nicknames Robert Seifer is <e-mail unknown>. He rotates IRC nicknames
Expand Down
8 changes: 5 additions & 3 deletions lib/POE/Kernel.pm
Expand Up @@ -1757,7 +1757,7 @@ sub alarm {
return EINVAL; return EINVAL;
} }


$self->_data_ev_clear_alarm_by_name($kr_active_session, $event_name); $self->_data_ev_clear_alarm_by_name($kr_active_session->ID(), $event_name);


# Add the new alarm if it includes a time. Calling _data_ev_enqueue # Add the new alarm if it includes a time. Calling _data_ev_enqueue
# directly is faster than calling alarm_set to enqueue it. # directly is faster than calling alarm_set to enqueue it.
Expand Down Expand Up @@ -1920,7 +1920,7 @@ sub alarm_remove {
} }


my ($time, $event) = my ($time, $event) =
$self->_data_ev_clear_alarm_by_id($kr_active_session, $alarm_id); $self->_data_ev_clear_alarm_by_id($kr_active_session->ID(), $alarm_id);
return unless defined $time; return unless defined $time;


# In a list context, return the alarm that was removed. In a scalar # In a list context, return the alarm that was removed. In a scalar
Expand Down Expand Up @@ -2058,7 +2058,9 @@ sub alarm_remove_all {
# Free every alarm owned by the session. This code is ripped off # Free every alarm owned by the session. This code is ripped off
# from the _stop code to flush everything. # from the _stop code to flush everything.


my @removed = $self->_data_ev_clear_alarm_by_session($kr_active_session); my @removed = $self->_data_ev_clear_alarm_by_session(
$kr_active_session->ID()
);


return unless defined wantarray; return unless defined wantarray;
return @removed if wantarray; return @removed if wantarray;
Expand Down
32 changes: 15 additions & 17 deletions lib/POE/Resource/Events.pm
Expand Up @@ -112,7 +112,7 @@ sub _data_ev_enqueue {


$self->_data_ses_refcount_inc($sid) unless $event_count{$sid}++; $self->_data_ses_refcount_inc($sid) unless $event_count{$sid}++;


return $new_id if $session == $source_session; return $new_id if $sid eq $source_session->ID();


$self->_data_ses_refcount_inc($source_session->ID) unless ( $self->_data_ses_refcount_inc($source_session->ID) unless (
$post_count{$source_session->ID}++ $post_count{$source_session->ID}++
Expand All @@ -124,18 +124,16 @@ sub _data_ev_enqueue {
### Remove events sent to or from a specific session. ### Remove events sent to or from a specific session.


sub _data_ev_clear_session { sub _data_ev_clear_session {
my ($self, $session) = @_; my ($self, $sid) = @_;

# TODO - Convert the event structure to SID too?


# Events sent to the session. # Events sent to the session.
PENDING: { PENDING: {
my $pending_count = $event_count{$session->ID}; my $pending_count = $event_count{$sid};
last PENDING unless $pending_count; last PENDING unless $pending_count;


foreach ( foreach (
$kr_queue->remove_items( $kr_queue->remove_items(
sub { $_[0][EV_SESSION] == $session }, sub { $_[0][EV_SESSION]->ID() eq $sid },
$pending_count $pending_count
) )
) { ) {
Expand All @@ -153,12 +151,12 @@ sub _data_ev_clear_session {


# Events sent by the session. # Events sent by the session.
SENT: { SENT: {
my $sent_count = $post_count{$session->ID}; my $sent_count = $post_count{$sid};
last SENT unless $sent_count; last SENT unless $sent_count;


foreach ( foreach (
$kr_queue->remove_items( $kr_queue->remove_items(
sub { $_[0][EV_SOURCE] == $session }, sub { $_[0][EV_SOURCE]->ID() eq $sid },
$sent_count $sent_count
) )
) { ) {
Expand All @@ -173,8 +171,8 @@ sub _data_ev_clear_session {
croak "lingering sent count: $sent_count" if $sent_count; croak "lingering sent count: $sent_count" if $sent_count;
} }


croak "lingering event count" if delete $event_count{$session->ID}; croak "lingering event count" if delete $event_count{$sid};
croak "lingering post count" if delete $post_count{$session->ID}; croak "lingering post count" if delete $post_count{$sid};
} }


# TODO Alarm maintenance functions may move out to a separate # TODO Alarm maintenance functions may move out to a separate
Expand All @@ -188,11 +186,11 @@ sub _data_ev_clear_session {
### future due times. ### future due times.


sub _data_ev_clear_alarm_by_name { sub _data_ev_clear_alarm_by_name {
my ($self, $session, $alarm_name) = @_; my ($self, $sid, $alarm_name) = @_;


my $my_alarm = sub { my $my_alarm = sub {
return 0 unless $_[0]->[EV_TYPE] & ET_ALARM; return 0 unless $_[0]->[EV_TYPE] & ET_ALARM;
return 0 unless $_[0]->[EV_SESSION] == $session; return 0 unless $_[0]->[EV_SESSION]->ID() eq $sid;
return 0 unless $_[0]->[EV_NAME] eq $alarm_name; return 0 unless $_[0]->[EV_NAME] eq $alarm_name;
return 1; return 1;
}; };
Expand All @@ -207,10 +205,10 @@ sub _data_ev_clear_alarm_by_name {
### times. TODO It's possible to remove non-alarms; is that wrong? ### times. TODO It's possible to remove non-alarms; is that wrong?


sub _data_ev_clear_alarm_by_id { sub _data_ev_clear_alarm_by_id {
my ($self, $session, $alarm_id) = @_; my ($self, $sid, $alarm_id) = @_;


my $my_alarm = sub { my $my_alarm = sub {
$_[0]->[EV_SESSION] == $session; $_[0]->[EV_SESSION]->ID() eq $sid;
}; };


my ($time, $id, $event) = $kr_queue->remove_item($alarm_id, $my_alarm); my ($time, $id, $event) = $kr_queue->remove_item($alarm_id, $my_alarm);
Expand All @@ -219,7 +217,7 @@ sub _data_ev_clear_alarm_by_id {
if (TRACE_EVENTS) { if (TRACE_EVENTS) {
_warn( _warn(
"<ev> removed event $id ``", $event->[EV_NAME], "'' to ", "<ev> removed event $id ``", $event->[EV_NAME], "'' to ",
$self->_data_alias_loggable($session->ID), " at $time" $self->_data_alias_loggable($sid), " at $time"
); );
} }


Expand All @@ -230,11 +228,11 @@ sub _data_ev_clear_alarm_by_id {
### Remove all the alarms for a session. Whoot! ### Remove all the alarms for a session. Whoot!


sub _data_ev_clear_alarm_by_session { sub _data_ev_clear_alarm_by_session {
my ($self, $session) = @_; my ($self, $sid) = @_;


my $my_alarm = sub { my $my_alarm = sub {
return 0 unless $_[0]->[EV_TYPE] & ET_ALARM; return 0 unless $_[0]->[EV_TYPE] & ET_ALARM;
return 0 unless $_[0]->[EV_SESSION] == $session; return 0 unless $_[0]->[EV_SESSION]->ID() eq $sid;
return 1; return 1;
}; };


Expand Down
5 changes: 4 additions & 1 deletion lib/POE/Resource/FileHandles.pm
Expand Up @@ -490,7 +490,10 @@ sub _data_handle_remove {


foreach ($kr_queue->remove_items($my_select)) { foreach ($kr_queue->remove_items($my_select)) {
my ($time, $id, $event) = @$_; my ($time, $id, $event) = @$_;
$self->_data_ev_refcount_dec( @$event[EV_SESSION, EV_SOURCE] ); $self->_data_ev_refcount_dec(
$event->[EV_SOURCE]->ID(),
$event->[EV_SESSION]->ID(),
);


TRACE_EVENTS and _warn( TRACE_EVENTS and _warn(
"<ev> removing select event $id ``$event->[EV_NAME]''" . "<ev> removing select event $id ``$event->[EV_NAME]''" .
Expand Down
2 changes: 1 addition & 1 deletion lib/POE/Resource/Sessions.pm
Expand Up @@ -226,7 +226,7 @@ sub _data_ses_free {
$self->_data_extref_clear_session($sid); # Remove all leftover extrefs. $self->_data_extref_clear_session($sid); # Remove all leftover extrefs.
$self->_data_handle_clear_session($sid); # Remove all leftover handles. $self->_data_handle_clear_session($sid); # Remove all leftover handles.


$self->_data_ev_clear_session($session); # Remove all leftover events. $self->_data_ev_clear_session($sid); # Remove all leftover events.


# Remove the session itself. # Remove the session itself.


Expand Down
12 changes: 6 additions & 6 deletions t/20_resources/00_base/events.pm
Expand Up @@ -118,7 +118,7 @@ check_references(
{ # Remove one of the alarms by its ID. { # Remove one of the alarms by its ID.


my ($time, $event) = $poe_kernel->_data_ev_clear_alarm_by_id( my ($time, $event) = $poe_kernel->_data_ev_clear_alarm_by_id(
$poe_kernel, $ids[1] $poe_kernel->ID(), $ids[1]
); );


is($time, 2, "removed event has the expected due time"); is($time, 2, "removed event has the expected due time");
Expand All @@ -136,7 +136,7 @@ check_references(
# did exist, except it doesn't. # did exist, except it doesn't.


my ($time, $event) = $poe_kernel->_data_ev_clear_alarm_by_id( my ($time, $event) = $poe_kernel->_data_ev_clear_alarm_by_id(
$poe_kernel, 8675309 $poe_kernel->ID(), 8675309
); );


ok(!defined($time), "can't clear bogus alarm by nonexistent ID"); ok(!defined($time), "can't clear bogus alarm by nonexistent ID");
Expand Down Expand Up @@ -166,7 +166,7 @@ is(
# Remove the alarm by name, for real. We should be down to one timer # Remove the alarm by name, for real. We should be down to one timer
# (the original poll thing). # (the original poll thing).


$poe_kernel->_data_ev_clear_alarm_by_name($poe_kernel, "timer"); $poe_kernel->_data_ev_clear_alarm_by_name($poe_kernel->ID(), "timer");
check_references( check_references(
$poe_kernel, 0, 0, 1, "after removing 'timer' by name" $poe_kernel, 0, 0, 1, "after removing 'timer' by name"
); );
Expand All @@ -181,7 +181,7 @@ check_references(
{ # Remove the last of the timers. The Kernel session is the only { # Remove the last of the timers. The Kernel session is the only
# reference left for it. # reference left for it.


my @removed = $poe_kernel->_data_ev_clear_alarm_by_session($poe_kernel); my @removed = $poe_kernel->_data_ev_clear_alarm_by_session($poe_kernel->ID());
is(@removed, 1, "removed the last alarm successfully"); is(@removed, 1, "removed the last alarm successfully");


# Verify that the removed timer is the correct one. We still have # Verify that the removed timer is the correct one. We still have
Expand Down Expand Up @@ -258,13 +258,13 @@ $poe_kernel->_data_ev_clear_session($poe_kernel);
$poe_kernel, 1, 1, 1, "after creating inter-session messages" $poe_kernel, 1, 1, 1, "after creating inter-session messages"
); );


$poe_kernel->_data_ev_clear_session($session); $poe_kernel->_data_ev_clear_session($session->ID());


check_references( check_references(
$poe_kernel, 1, 0, 0, "after clearing inter-session messages" $poe_kernel, 1, 0, 0, "after clearing inter-session messages"
); );


$poe_kernel->_data_ev_clear_session($poe_kernel); $poe_kernel->_data_ev_clear_session($poe_kernel->ID());


check_references( check_references(
$poe_kernel, 1, 0, 0, "after clearing kernel messages" $poe_kernel, 1, 0, 0, "after clearing kernel messages"
Expand Down

0 comments on commit 9654581

Please sign in to comment.