Further testing has uncovered a couple of other bugs which occur when
one connector closes unexpectedly:
1. If the write buffer associated with a particular connector contains
data at the instant that the remote host closes the socket, then the
local socket will never be closed. This is because close_sockets() does
a test for "if( my $data = Net::Proxy->get_buffer( $sock ) )" and if
true it does not attempt to close the socket. The intention is
obviously to try and close the connection gracefully by waiting until
the data from the buffer is written to the remote host. However, if the
remote host has closed the socket then this is impossible. To fix this:
a) raw_read_from() should clear the buffer before closing
close_socket(). Note that it should only clear the buffer of $sock (and
not $peer) because $peer may still be connected and can therefore write
out any data remaining in it's write buffer.
b) add_to_buffer() should not allow any more data to be appended to the
buffer if the connector is in the CLOSING list. If any additional data
is received from $peer then it will be added to the write buffer of
$sock. If the remote host has closed the socket then the data can never
be written. Therefore the buffer will never become empty and the
get_buffer() test mentioned above will always be true, meaning that the
local socket is never closed.
2. close_sockets() needs to handle being called on a socket which has
already been closed. This can occur in the following situation:
* $sock1 closes connection
* raw_read_from() calls close_sockets($sock1, $sock2) to close both
connectors
* $sock1 has empty buffer so it is closed properly
* $sock2 still has data in associated write buffer so there is no
attempt to close it but it is added to CLOSING list
... control returns to mainloop() to wait for remaining data to be
written to $sock2 ...
* remote host on $sock2 closes connection before all data is finished
writing
* raw_read_from() calls close_socket($sock2, $sock)
$sock2 will be now be closed, but $sock1 was closed already!
close_sockets() already has a check for "if ( my $conn =
Net::Proxy->get_connector($sock) )" but this is done *after* the call to
get_nick($sock1) (which will return undef). This can be fixed by doing
the get_connector() test earlier in the loop. I can't see any reason
why it cannot go at the very top (the internal structures should not
need to be cleaned up for sockets which have already been closed).
The attached patch fixes these two problems as well as the one outlined
in my previous e-mail.
diff -ruN Net-Proxy-0.12.orig//lib/Net/Proxy/Connector.pm Net-Proxy-0.12/lib/Net/Proxy/Connector.pm
--- Net-Proxy-0.12.orig//lib/Net/Proxy/Connector.pm 2007-10-18 00:57:44.000000000 +0200
+++ Net-Proxy-0.12/lib/Net/Proxy/Connector.pm 2010-08-26 05:51:34.181446107 +0200
@@ -126,6 +126,10 @@
# connection closed
if ( $close || $read == 0 ) {
+ # $sock was closed either forcefully or gracefully, either way
+ # we have no chance to send remaining data
+ Net::Proxy->set_buffer( $sock, '' );
+
my $peer = Net::Proxy->get_peer($sock);
$self->get_proxy()->close_sockets( $sock, $peer );
return;
diff -ruN Net-Proxy-0.12.orig//lib/Net/Proxy.pm Net-Proxy-0.12/lib/Net/Proxy.pm
--- Net-Proxy-0.12.orig//lib/Net/Proxy.pm 2007-10-18 00:57:44.000000000 +0200
+++ Net-Proxy-0.12/lib/Net/Proxy.pm 2010-08-26 05:51:10.513445908 +0200
@@ -104,7 +104,7 @@
$n++;
}
# special shortcut
- sub add_to_buffer { $SOCK_INFO{ refaddr $_[1] }[$buffer_id] .= $_[2]; }
+ sub add_to_buffer { $SOCK_INFO{ refaddr $_[1] }[$buffer_id] .= $_[2] if(!exists($CLOSING{$_[1]})); }
}
#
@@ -137,6 +137,9 @@
SOCKET:
for my $sock (@socks) {
+ my $conn = Net::Proxy->get_connector($sock);
+ next SOCKET if(!defined($conn));
+
if( my $data = Net::Proxy->get_buffer( $sock ) ) {
## Net::Proxy->debug( length($data) . ' bytes left to write on ' . Net::Proxy->get_nick( $sock ) );
$CLOSING{ refaddr $sock} = $sock;
@@ -146,16 +149,14 @@
Net::Proxy->notice( 'Closing ' . Net::Proxy->get_nick( $sock ) );
# clean up connector
- if ( my $conn = Net::Proxy->get_connector($sock) ) {
- $conn->close($sock) if $conn->can('close');
+ $conn->close($sock) if $conn->can('close');
- # count connections to the proxy "in connectors" only
- my $proxy = $conn->get_proxy();
- if ( refaddr $conn == refaddr $proxy->in_connector()
- && !_is_listener($sock) )
- {
- $proxy->stat_inc_closed();
- }
+ # count connections to the proxy "in connectors" only
+ my $proxy = $conn->get_proxy();
+ if ( refaddr $conn == refaddr $proxy->in_connector()
+ && !_is_listener($sock) )
+ {
+ $proxy->stat_inc_closed();
}
# clean up internal structures
@@ -278,6 +279,7 @@
# then write
for my $sock (@{$ready[1]}) {
my $conn = Net::Proxy->get_connector($sock);
+ next if(!defined($conn)); # may happen if read_from() closed socket
$conn->write_to($sock);
}
