Attached is a patch for high_perf branch:
PostLoopCallback was a global and would be overwritten by anyone with an
interest. This caused a race condition. For example, check_earlytalker
would fail horribly if there was more than one connection.
Includes a revert on my earlier check_earlytalker kludge, since this patch
fixes the cause and the older one only addressed the symptom.
This seems to work for me, but I have some lingering questions.
Open questions:
I retained the global $PostLoopCallback. Others will be better at judging
whether it can be completely removed. In particular, the $class vs $self
semantic of SetPostLoopCallback() in Socket.pm made me cautious.
This patch supports one PostLoopCallback at a time per connection. Is that
enough?
When this loops through all the PostLoopCallbacks, it "return unless"'s on
each. Can they all be safely run, then return with 0 if any were 0?
Brian
PostLoopCallback was a global and would be overwritten by anyone with an
interest. This caused a race condition. For example, check_earlytalker
would fail horribly if there was more than one connection.
Includes a revert on my earlier check_earlytalker kludge, since this patch
fixes the cause and the older one only addressed the symptom.
This seems to work for me, but I have some lingering questions.
Open questions:
I retained the global $PostLoopCallback. Others will be better at judging
whether it can be completely removed. In particular, the $class vs $self
semantic of SetPostLoopCallback() in Socket.pm made me cautious.
This patch supports one PostLoopCallback at a time per connection. Is that
enough?
When this loops through all the PostLoopCallbacks, it "return unless"'s on
each. Can they all be safely run, then return with 0 if any were 0?
--- old/lib/Danga/Client.pm 2005-05-05 03:32:07.000000000 -0600
+++ new/lib/Danga/Client.pm 2005-05-07 14:26:39.000000000 -0600
@@ -124,6 +124,7 @@
my Danga::Client $self = shift;
$self->{closing} = 1;
print "closing @_\n" if $::DEBUG;
+ $self->SetPostLoopCallback(undef);
$self->SUPER::close(@_);
}
--- old/lib/Danga/Socket.pm 2005-05-05 03:32:07.000000000 -0600
+++ new/lib/Danga/Socket.pm 2005-05-07 14:23:33.000000000 -0600
@@ -25,6 +25,7 @@
use fields qw(sock fd write_buf write_buf_offset write_buf_size
read_push_back
+ PostLoopCallback
closed event_watch debug_level);
use Errno qw(EINPROGRESS EWOULDBLOCK EISCONN
@@ -307,9 +308,22 @@
# now we can close sockets that wanted to close during our event
processing.
# (we didn't want to close them during the loop, as we didn't want fd
numbers
# being reused and confused during the event loop)
- $_->close while ($_ = shift @ToClose);
+ while(my $j = shift @ToClose) {
+ $j->[1]->close();
+ $j->[0]->{closing} = 0;
+ }
+
+ # now we're at the very end, call per-connection callbacks if defined
+ for my $fd (%DescriptorMap) {
+ my $sef = $DescriptorMap{$fd};
+ if( exists $sef->{PostLoopCallback} ) {
+ if( defined $sef->{PostLoopCallback} ) {
+ return unless $sef->{PostLoopCallback}->(\%DescriptorMap,
\%OtherFds);
+ }
+ }
+ }
- # now we're at the very end, call callback if defined
+ # now we're at the very end, call global callback if defined
if (defined $PostLoopCallback) {
return $PostLoopCallback->(\%DescriptorMap, \%OtherFds);
}
@@ -472,7 +486,7 @@
# defer closing the actual socket until the event loop is done
# processing this round of events. (otherwise we might reuse fds)
- push @ToClose, $sock;
+ push @ToClose, [$self,$sock];
return 0;
}
@@ -785,7 +799,18 @@
### be passed two parameters: \%DescriptorMap, \%OtherFds.
sub SetPostLoopCallback {
my ($class, $ref) = @_;
- $PostLoopCallback = (defined $ref && ref $ref eq 'CODE') ? $ref : undef;
+ if(ref $class) {
+ my Danga::Socket $self = $class;
+ if( defined $ref && ref $ref eq 'CODE' ) {
+ $self->{PostLoopCallback} = $ref;
+ }
+ else {
+ delete $self->{PostLoopCallback} ;
+ }
+ }
+ else {
+ $PostLoopCallback = (defined $ref && ref $ref eq 'CODE') ? $ref :
undef;
+ }
}
#####################################################################
--- old/plugins/check_earlytalker 2005-05-05 03:32:07.000000000 -0600
+++ new/plugins/check_earlytalker 2005-05-07 14:56:56.000000000 -0600
@@ -44,8 +44,6 @@
=cut
-use Time::HiRes ();
-
use warnings;
use strict;
@@ -70,16 +68,8 @@
sub connect_handler {
my ($self, $transaction) = @_;
- my $qp = $self->qp;
- my $end = Time::HiRes::time + $self->{_args}->{'wait'} ;
- my $time;
- for( $time = Time::HiRes::time; $time < $end && !length($qp->{line}) ; $time
= Time::HiRes::time ) {
- $qp->can_read($end-$time);
- }
- my $earlytalker = 0;
- $earlytalker = 1 if $time < $end ;
- if ($earlytalker) {
+ if ($self->qp->can_read($self->{_args}->{'wait'})) {
$self->log(LOGNOTICE, 'remote host started talking before we said hello');
if ($self->{_args}->{'defer-reject'}) {
$self->connection->notes('earlytalker', 1);
