Fixed memory leak in NRD::Client, with test to catch in future

Author: tonvoon

Changes

@@ -1,4 +1,4 @@
-Revision history for Perl module NSCA2::Daemon
+Revision history for Perl module NRD::Daemon
 
 0.01 Fri Jul  3 18:49:18 2009
     - original version; created by ExtUtils::ModuleMaker 0.51

Makefile.PL

@@ -19,5 +19,7 @@ WriteMakefile(
 		     'Crypt::CBC' => 0,       # for testing. optional for operation
                      'Crypt::Blowfish' => 0,  # for testing. optional for operation
 		     'JSON::XS' => 0,
+                     'Test::Memory::Cycle' => 1.04,  # for testing only
+                     'Scalar::Util' => 1.23,
                     },
 );

README

@@ -1,4 +1,4 @@
-pod2text NSCA2::Daemon.pm > README
+pod2text NRD::Daemon.pm > README
 
 If this is still here it means the programmer was too lazy to create the readme file.
 

Todo

@@ -1,4 +1,4 @@
-TODO list for Perl module NSCA2::Daemon
+TODO list for Perl module NRD::Daemon
 
 - Streaming lots of results
 

lib/NRD/Client.pm

@@ -7,6 +7,7 @@ use IO::Socket;
 use NRD::Packet;
 use NRD::Serialize;
 use Carp;
+use Scalar::Util qw(weaken);
 
 =item new( \%options )
 
@@ -92,11 +93,15 @@ sub connect {
         if ($self->{timeout}) {
             alarm( $self->{timeout} );
         }
-        print $sock $data;
+        my $s = $self->{sock};
+        print $s $data;
         if ($self->{timeout}) {
             alarm(0);
         }
     };
+    # Need to weaken $self as it is in the send_sock routine. Tested in t/005_client.t
+    weaken($self);  
+
     $self->{sock} = $sock;
     if ($self->{serializer}->needs_helo) {
         $self->{send_sock}->($self->{packer}->pack( $self->{serializer}->helo ));
@@ -158,6 +163,10 @@ sub end {
     };
     my $error = $@;
     close $self->{sock};
+
+    # This undef is not strictly necessary, but it stops Memory::Cycle from erroring re: the GLOB variable
+    $self->{sock} = undef;
+
     alarm(0) if $self->{timeout};
     croak $error if $error;
 

t/005_client.t

@@ -0,0 +1,67 @@
+#!/usr/bin/perl
+#
+# DESCRIPTION:
+#	Test client for memory leaks
+#
+# LICENCE:
+#	GNU GPLv2
+
+use lib 't';
+
+use strict;
+use NSCATest;
+use Test::More;
+use Net::Server::Fork;
+use Test::Memory::Cycle;
+
+plan tests => 2;
+
+my $host = 'localhost';
+my $port = 7669;
+
+
+my $nsca = NSCATest->new( config => "plain" );
+$nsca->start("--server_type=Single");
+
+sleep 1;
+
+my $data = [ 
+	["hostname", "0", "Plugin output"],
+	["long_output", 0, 'x' x 10240 ],
+	["hostname-with-other-bits", "1", "More data to be read"],
+	["hostname.here", "2", "Check that ; are okay to receive"],
+	["host", "service", 0, "A good result here"],
+	["host54", "service with spaces", 1, "Warning! My flies are undone!"],
+	["host-robin", "service with a :)", 2, "Critical? Alert! Alert!"],
+	["host-batman", "another service", 3, "Unknown - the only way to travel"],
+	["long_output", "service1", 0, 'x' x 10240 ], #10K of plugin output
+	['x' x 1000, 0, 'Host check with big hostname'],
+	['x' x 1000, 'service OK', 0, 'Service check with big hostname'],
+	['long_svc_name', 'x' x 1000, 0, 'Service check with big service name'],
+	];
+
+# When the NRD server cuts a connection we get a SIG_PIPE. Perls default is to die...
+$SIG{'PIPE'} = 'IGNORE';
+
+use NRD::Client;
+
+my $client;
+eval { 
+    $client = NRD::Client->new( { timeout => 5, serializer => "plain" } );
+    $client->connect( PeerAddr => "127.0.0.1", PeerPort => $port, Proto => "tcp" );
+    foreach my $d ( @$data ) {
+        $client->send_result( { 
+            command => "result", 
+            data => { 
+                time => time(), 
+                %{ $client->{serializer}->from_line( join("\t", @$d) ) },
+            },
+        } );
+    }
+    $client->end;
+};
+is( $@, '', "No errors" );
+memory_cycle_ok( $client, "No memory cycles for client" );
+
+$nsca->stop;
+

t/send_digest.cfg

@@ -1,5 +1,5 @@
 ####################################################
-# Sample NSCA2 Client Config File 
+# Sample NRD Client Config File 
 ####################################################
 
 host 127.0.0.1

t/send_encrypt.cfg

@@ -1,5 +1,5 @@
 ####################################################
-# Sample NSCA2 Client Config File 
+# Sample NRD Client Config File 
 ####################################################
 
 host 127.0.0.1

t/send_plain.cfg

@@ -1,5 +1,5 @@
 ####################################################
-# Sample NSCA2 Client Config File 
+# Sample NRD Client Config File 
 ####################################################
 
 host 127.0.0.1

t/send_resultdir.cfg

@@ -1,5 +1,5 @@
 ####################################################
-# Sample NSCA2 Client Config File 
+# Sample NRD Client Config File 
 ####################################################
 
 host 127.0.0.1