Subject: | Race condition in DBM::Deep::File::_lock() |
Date: | Thu, 12 May 2016 15:58:03 +0200 |
To: | bug-DBM-Deep [...] rt.cpan.org |
From: | Nick Hibma <nick [...] van-laarhoven.org> |
Gents,
There is a race condition in the following code:
sub _lock {
...
if ($self->{locking}) {
if (!$self->{locked}) {
flock($self->{fh}, $type);
# refresh end counter in case file has changed size
my @stats = stat($self->{fh});
$self->{end} = $stats[7];
# double-check file inode, in case another process
# has optimize()d our file while we were waiting.
if (defined($self->{inode}) && $stats[1] != $self->{inode}) {
$self->close;
$self->open;
#XXX This needs work
$obj->{engine}->setup( $obj );
flock($self->{fh}, $type); # re-lock
# This may not be necessary after re-opening
$self->{end} = (stat($self->{fh}))[7]; # re-end
}
}
…
}
If the file is optimised twice in rapid succession, the second flock will lock the wrong file. I’d suggest repeating until the lock, end stats and node check succeeds (note: code is untested!):
sub _lock {
my $self = shift;
my ($obj, $type) = @_;
$type = LOCK_EX unless defined $type;
#XXX This is a temporary fix for Win32 and autovivification. It
# needs to improve somehow. -RobK, 2008-03-09
if ( $^O eq 'MSWin32' || $^O eq 'cygwin' ) {
$type = LOCK_EX;
}
if (!defined($self->{fh})) { return; }
#XXX This either needs to allow for upgrading a shared lock to an
# exclusive lock or something else with autovivification.
# -RobK, 2008-03-09
if ($self->{locking}) {
while (!$self->{locked}) {
flock($self->{fh}, $type);
# refresh end counter in case file has changed size
my @stats = stat($self->{fh});
$self->{end} = $stats[7];
# double-check file inode, in case another process
# has optimize()d our file while we were waiting.
if (defined($self->{inode}) && $stats[1] != $self->{inode}) {
$self->close;
$self->open;
#XXX This needs work
$obj->{engine}->setup( $obj );
} else {
$self->{locked}++;
}
}
return 1;
}
return;
}
Kind regards,
Nick