Merge pull request #1717 from connortechnology/fix_mmap_leak

must call zmMemInvalidate before next

scripts/ZoneMinder/lib/ZoneMinder/Memory.pm.in

@@ -180,7 +180,6 @@ our $mem_data = {
 };
 
 our $mem_size = 0;
-our $mem_verified = {};
 
 sub zmMemInit {
   my $offset = 0;
@@ -249,8 +248,6 @@ sub zmMemVerify {
     return( undef );
   }
 
-  my $mem_key = zmMemKey( $monitor );
-  if ( !defined($mem_verified->{$mem_key}) ) {
     my $sd_size = zmMemRead( $monitor, "shared_data:size", 1 );
     if ( $sd_size != $mem_data->{shared_data}->{size} ) {
       if ( $sd_size ) {
@@ -292,8 +289,11 @@ sub zmMemVerify {
       }
       return( undef );
     }
-    $mem_verified->{$mem_key} = !undef;
-  }
+    if ( !zmMemRead($monitor, "shared_data:valid",1) ) {
+        Error( "Shared data not valid for monitor $$monitor{Id}" );
+        return( undef );
+    }
+
   return( !undef );
 }
 
@@ -366,7 +366,6 @@ sub zmMemInvalidate {
   my $monitor = shift;
   my $mem_key = zmMemKey($monitor);
   if ( $mem_key ) {
-    delete $mem_verified->{$mem_key};
     zmMemDetach( $monitor );
   }
 }

scripts/ZoneMinder/lib/ZoneMinder/Memory/Mapped.pm

@@ -104,20 +104,21 @@ sub zmMemAttach
             );
             return ( undef );
         }
-        if ( !open( MMAP, "+<", $mmap_file ) )
+        my $MMAP;
+        if ( !open( $MMAP, "+<", $mmap_file ) )
         {
             Error( sprintf( "Can't open memory map file '%s': $!\n", $mmap_file ) );
             return( undef );
         }
         my $mmap = undef;
-        my $mmap_addr = mmap( $mmap, $size, PROT_READ|PROT_WRITE, MAP_SHARED, \*MMAP );
+        my $mmap_addr = mmap( $mmap, $size, PROT_READ|PROT_WRITE, MAP_SHARED, $MMAP );
         if ( !$mmap_addr || !$mmap )
         {
             Error( sprintf( "Can't mmap to file '%s': $!\n", $mmap_file ) );
-            close( MMAP );
+            close( $MMAP );
             return( undef );
         }
-        $monitor->{MMapHandle} = \*MMAP;
+        $monitor->{MMapHandle} = $MMAP;
         $monitor->{MMapAddr} = $mmap_addr;
         $monitor->{MMap} = \$mmap;
     }

scripts/zmtrigger.pl.in

@@ -196,8 +196,10 @@ foreach my $connection ( @in_select_connections )
 
 my %spawned_connections;
 my %monitors;
-
 my $monitor_reload_time = 0;
+my $needsReload = 0;
+loadMonitors();
+
 
 $! = undef;
 my $rin = '';
@@ -313,6 +315,14 @@ while( 1 )
     my @out_messages;
     foreach my $monitor ( values(%monitors) )
     {
+
+        if ( ! zmMemVerify($monitor) ) {
+          # Our attempt to verify the memory handle failed. We should reload the monitors.
+          # Don't need to zmMemInvalidate because the monitor reload will do it.
+          $needsReload = 1;
+          next;
+        }
+
         my ( $state, $last_event )
             = zmMemRead( $monitor,
                          [ "shared_data:state",
@@ -413,7 +423,7 @@ while( 1 )
     }
 
     # If necessary reload monitors
-    if ( (time() - $monitor_reload_time) > MONITOR_RELOAD_INTERVAL )
+    if ( $needsReload || ((time() - $monitor_reload_time) > MONITOR_RELOAD_INTERVAL ))
     {
         foreach my $monitor ( values(%monitors) )
         {
@@ -421,6 +431,7 @@ while( 1 )
             zmMemInvalidate( $monitor );
         }
         loadMonitors();
+        $needsReload = 0;
     }
 }
 Info( "Trigger daemon exiting\n" );
@@ -443,7 +454,11 @@ sub loadMonitors
         or Fatal( "Can't execute: ".$sth->errstr() );
     while( my $monitor = $sth->fetchrow_hashref() )
     {
-        next if ( !zmMemVerify( $monitor ) ); # Check shared memory ok
+          # Check shared memory ok
+          if ( !zmMemVerify( $monitor ) ) {
+            zmMemInvalidate( $monitor );
+            next;
+          }
 
         if ( defined($monitors{$monitor->{Id}}->{LastState}) )
         {

scripts/zmwatch.pl.in

@@ -87,24 +87,19 @@ while( 1 )
     {
         next if $monitor->{Function} eq 'None';
         my $restart = 0;
-        # Prevent open handles building up if we have connect to shared memory
-        # Many of our error checks below do a next without closing the mem handle.
-        # zmMemInvalidate will just return of nothing is open, so we can just do this here.
-        zmMemInvalidate( $monitor );
-        if ( zmMemVerify( $monitor )
-             && zmMemRead( $monitor, "shared_data:valid" )
-        )
-        {
+        if ( zmMemVerify( $monitor ) ) {
             # Check we have got an image recently
             my $image_time = zmGetLastWriteTime( $monitor );
             if ( !defined($image_time) ) {
 				 # Can't read from shared data
 				Debug( "LastWriteTime is not defined." );
+        zmMemInvalidate( $monitor );
 				next;
 			}
             if ( !$image_time ) {
 				# We can't get the last capture time so can't be sure it's died.
 				Debug( "LastWriteTime is = $image_time." );
+        zmMemInvalidate( $monitor );
 				next;
 			}
 

scripts/zmx10.pl.in

@@ -474,7 +474,11 @@ sub loadTasks
         or Fatal( "Can't execute: ".$sth->errstr() );
     while( my $monitor = $sth->fetchrow_hashref() )
     {
-        next if ( !zmMemVerify( $monitor ) ); # Check shared memory ok
+# Check shared memory ok
+      if ( !zmMemVerify( $monitor ) ) {
+        zmMemInvalidate( $monitor );
+        next ;
+      }
 
         $monitor_hash{$monitor->{Id}} = $monitor;