From: jhuckaby <jhuckaby@50811bd7-b8ce-0310-adc1-d9db26280581>
Date: Thu, 23 Feb 2006 07:47:10 +0000 (+0000)
Subject: Fixed optimize() bug with locking and added tests.
X-Git-Tag: 0-97~14
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=a59a8dcae9c7b56818d5f20d73a2b61b3993ae7e;p=dbsrgits%2FDBM-Deep.git

Fixed optimize() bug with locking and added tests.
---

diff --git a/lib/DBM/Deep.pm b/lib/DBM/Deep.pm
index 977fa97..452270d 100644
--- 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
--- 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;