Fixed optimize() bug with locking and added tests.

diff --git a/lib/DBM/Deep.pm b/lib/DBM/Deep.pm
index 977fa97..452270d 100644 (file)
--- a/lib/DBM/Deep.pm
+++ b/lib/DBM/Deep.pm
@@ -279,7 +279,9 @@ sub _open {
         return $self->_throw_error("Signature not found -- file is not a Deep DB");
     }
 
-    $self->root->{end} = (stat($fh))[7];
+       my @stats = stat($fh);
+       $self->root->{inode} = $stats[1];
+    $self->root->{end} = $stats[7];
         
     ##
     # Get our type from master index signature
@@ -919,7 +921,16 @@ sub lock {
        if (!defined($self->fh)) { return; }
 
        if ($self->root->{locking}) {
-               if (!$self->root->{locked}) { flock($self->fh, $type); }
+               if (!$self->root->{locked}) {
+                       flock($self->fh, $type);
+                       
+                       # double-check file inode, in case another process
+                       # has optimize()d our file while we were waiting.
+                       if ((stat($self->root->{file}))[1] != $self->root->{inode}) {
+                               $self->_open(); # re-open
+                               flock($self->fh, $type); # re-lock
+                       }
+               }
                $self->root->{locked}++;
 
         return 1;
@@ -1284,13 +1295,13 @@ sub STORE {
                return;
        }
        ##
-
-    my $fh = $self->fh;
        
        ##
        # Request exclusive lock for writing
        ##
        $self->lock( LOCK_EX );
+       
+       my $fh = $self->fh;
 
        ##
        # If locking is enabled, set 'end' parameter again, in case another

diff --git a/t/11_optimize.t b/t/11_optimize.t
index 0562597..86a8ac3 100644 (file)
--- a/t/11_optimize.t
+++ b/t/11_optimize.t
@@ -2,7 +2,7 @@
 # DBM::Deep Test
 ##
 use strict;
-use Test::More tests => 5;
+use Test::More tests => 9;
 
 use_ok( 'DBM::Deep' );
 
@@ -53,8 +53,87 @@ if ($db->error()) {
        die "ERROR: " . $db->error();
 }
 
-ok( $result );
-ok( $after < $before ); # make sure file shrunk
+ok( $result, "optimize succeeded" );
+ok( $after < $before, "file size has shrunk" ); # make sure file shrunk
 
 is( $db->{key1}, 'value1', "key1's value is still there after optimize" );
 is( $db->{a}{c}, 'value2', "key2's value is still there after optimize" );
+
+##
+# now for the tricky one -- try to store a new key while file is being
+# optimized and locked by another process.  filehandle should be invalidated, 
+# and automatically re-opened transparently.  Cannot test on Win32, due to 
+# problems with fork()ing, flock()ing, etc.  Win32 very bad.
+##
+
+if ( $^O eq 'MSWin32' ) {
+       ok(1, "Skipping test on this platform");
+       ok(1, "Skipping test on this platform");
+       ok(1, "Skipping test on this platform");
+       ok(1, "Skipping test on this platform");
+       exit(1);
+}
+
+##
+# first things first, get us about 1000 keys so the optimize() will take 
+# at least a few seconds on any machine, and re-open db with locking
+##
+for (1..1000) { $db->STORE( $_, $_ ); }
+undef $db;
+
+##
+# now, fork a process for the optimize()
+##
+my $pid = fork();
+ok( defined($pid), "fork was successful" ); # make sure fork was successful
+
+if ($pid) {
+       # parent fork
+       
+       # re-open db
+       $db = DBM::Deep->new(
+               file => "t/test.db",
+               autoflush => 1,
+               locking => 1
+       );
+       if ($db->error()) {
+               die "ERROR: " . $db->error();
+       }
+       
+       # sleep for 1 second to make sure optimize() is running in the other fork
+       sleep(1);
+       
+       # now, try to get a lock and store a key
+       $db->{parentfork} = "hello";
+       
+       # see if it was stored successfully
+       is( $db->{parentfork}, "hello", "stored key while optimize took place" );
+       # ok(1);
+       
+       # now check some existing values from before
+       is( $db->{key1}, 'value1', "key1's value is still there after optimize" );
+       is( $db->{a}{c}, 'value2', "key2's value is still there after optimize" );
+}
+else {
+       # child fork
+       
+       # re-open db
+       $db = DBM::Deep->new(
+               file => "t/test.db",
+               autoflush => 1,
+               locking => 1
+       );
+       if ($db->error()) {
+               die "ERROR: " . $db->error();
+       }
+       
+       # optimize and exit
+       $db->optimize();
+
+       ok(1, "Ignore this, we're in a fork");
+       ok(1, "Ignore this, we're in a fork");
+       ok(1, "Ignore this, we're in a fork");
+       exit(0);
+}
+
+1;