Skip to content

Commit

Permalink
Fix spurious file locking problems with precomp store
Browse files Browse the repository at this point in the history
File locking is a mess. Because it's racy, we cannot rely on upgrading a
shared lock to an exclusive lock, so we have to take an exclusive lock
even for reading precomp files.

We also now only lock a file once and count how often we tried locking
the same file, so we only unlock after the last locker set it free.

Lastly we re-check the existence of a precomp file after loading failed
and we locked the store again for precompilation, because in between,
someone else may have precompiled and we could only make matters worse
if we precompile again.
  • Loading branch information
niner committed Dec 10, 2015
1 parent 7df9978 commit 67f9259
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
7 changes: 6 additions & 1 deletion src/core/CompUnit/PrecompilationRepository.pm
Expand Up @@ -56,12 +56,17 @@ class CompUnit::PrecompilationRepository::Default does CompUnit::PrecompilationR
}
}
else {
self.store.unlock;
CompUnit::Handle
}
}

method precompile(IO::Path:D $path, CompUnit::PrecompilationId $id) {
method precompile(IO::Path:D $path, CompUnit::PrecompilationId $id, Bool :$force = False) {
my $io = self.store.destination($*PERL.compiler.id, $id);
if not $force and $io.e and $io.s {
self.store.unlock;
return True;
}

my Mu $opts := nqp::atkey(%*COMPILING, '%?OPTIONS');
my $lle = !nqp::isnull($opts) && !nqp::isnull(nqp::atkey($opts, 'll-exception'))
Expand Down
15 changes: 10 additions & 5 deletions src/core/CompUnit/PrecompilationStore/File.pm
@@ -1,6 +1,7 @@
class CompUnit::PrecompilationStore::File does CompUnit::PrecompilationStore {
has IO::Path $.prefix is required;
has IO::Handle $!lock;
has int $!lock-count = 0;

submethod BUILD(IO::Path :$!prefix) {
$!prefix.mkdir;
Expand All @@ -20,22 +21,26 @@ class CompUnit::PrecompilationStore::File does CompUnit::PrecompilationStore {
self!dir($compiler-id, $precomp-id).child($precomp-id.IO)
}

method !lock(Int:D $mode) {
method !lock() {
return if $*W && $*W.is_precompilation_mode();
$!lock //= $.prefix.child('.lock').open(:create, :rw);
$!lock.lock($mode);
$!lock.lock(2) if $!lock-count == 0;
$!lock-count++;
}

method unlock() {
$!lock ?? $!lock.unlock !! True;
return if $*W && $*W.is_precompilation_mode();
die "unlock when we're not locked!" if $!lock-count == 0;
$!lock-count-- if $!lock-count > 0;
$!lock && $!lock-count == 0 ?? $!lock.unlock !! True;
}

method load(CompUnit::PrecompilationId $compiler-id,
CompUnit::PrecompilationId $precomp-id)
{
self!lock();
my $path = self.path($compiler-id, $precomp-id);
if $path ~~ :e {
self!lock(1);
$path
}
else {
Expand All @@ -47,7 +52,7 @@ class CompUnit::PrecompilationStore::File does CompUnit::PrecompilationStore {
CompUnit::PrecompilationId $precomp-id)
returns IO::Path
{
self!lock(2);
self!lock();
my $compiler-dir = self.prefix.child($compiler-id.IO);
$compiler-dir.mkdir unless $compiler-dir.e;
my $dest = self!dir($compiler-id, $precomp-id);
Expand Down
4 changes: 2 additions & 2 deletions src/core/CompUnit/Repository/Installation.pm
Expand Up @@ -178,10 +178,10 @@ sub MAIN(:$name is copy, :$auth, :$ver, *@, *%) {
my $rev-deps-file = ($precomp.store.path($*PERL.compiler.id, $id) ~ '.rev-deps').IO;
my @rev-deps = $rev-deps-file.e ?? $rev-deps-file.lines !! ();

$precomp.precompile($source.IO, $id);
$precomp.precompile($source.IO, $id, :force);
for @rev-deps -> $rev-dep-id {
my $source = $sources-dir.child($rev-dep-id);
$precomp.precompile($source, $rev-dep-id) if $source.e;
$precomp.precompile($source, $rev-dep-id, :force) if $source.e;
}
}
}
Expand Down

0 comments on commit 67f9259

Please sign in to comment.