install : finer graned locking for install command

5 files changed, 97 insertions, 30 deletions

diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py
index f5f53101ae..2cde389bd2 100644
--- a/lib/spack/llnl/util/lock.py
+++ b/lib/spack/llnl/util/lock.py
@@ -28,9 +28,13 @@ import errno
 import time
 import socket
 
+import llnl.util.tty as tty
+
+
 __all__ = ['Lock', 'LockTransaction', 'WriteTransaction', 'ReadTransaction',
            'LockError']
 
+
 # Default timeout in seconds, after which locks will raise exceptions.
 _default_timeout = 60
 
@@ -120,6 +124,7 @@ class Lock(object):
 
         """
         if self._reads == 0 and self._writes == 0:
+            tty.debug('READ LOCK : {0._file_path} [Acquiring]'.format(self))
             self._lock(fcntl.LOCK_SH, timeout)   # can raise LockError.
             self._reads += 1
             return True
@@ -139,6 +144,7 @@ class Lock(object):
 
         """
         if self._writes == 0:
+            tty.debug('WRITE LOCK : {0._file_path} [Acquiring]'.format(self))
             self._lock(fcntl.LOCK_EX, timeout)   # can raise LockError.
             self._writes += 1
             return True
@@ -159,6 +165,7 @@ class Lock(object):
         assert self._reads > 0
 
         if self._reads == 1 and self._writes == 0:
+            tty.debug('READ LOCK : {0._file_path} [Released]'.format(self))
             self._unlock()      # can raise LockError.
             self._reads -= 1
             return True
@@ -179,6 +186,7 @@ class Lock(object):
         assert self._writes > 0
 
         if self._writes == 1 and self._reads == 0:
+            tty.debug('WRITE LOCK : {0._file_path} [Released]'.format(self))
             self._unlock()      # can raise LockError.
             self._writes -= 1
             return True
diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index e51024b05f..70abe1dd00 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -84,15 +84,14 @@ def install(parser, args):
     specs = spack.cmd.parse_specs(args.packages, concretize=True)
     for spec in specs:
         package = spack.repo.get(spec)
-        with spack.installed_db.write_transaction():
-            package.do_install(
-                keep_prefix=args.keep_prefix,
-                keep_stage=args.keep_stage,
-                install_deps=not args.ignore_deps,
-                install_self=not args.deps_only,
-                make_jobs=args.jobs,
-                run_tests=args.run_tests,
-                verbose=args.verbose,
-                fake=args.fake,
-                dirty=args.dirty,
-                explicit=True)
+        package.do_install(
+            keep_prefix=args.keep_prefix,
+            keep_stage=args.keep_stage,
+            install_deps=not args.ignore_deps,
+            install_self=not args.deps_only,
+            make_jobs=args.jobs,
+            run_tests=args.run_tests,
+            verbose=args.verbose,
+            fake=args.fake,
+            dirty=args.dirty,
+            explicit=True)
diff --git a/lib/spack/spack/database.py b/lib/spack/spack/database.py
index f73d3765c8..8c29ceeb27 100644
--- a/lib/spack/spack/database.py
+++ b/lib/spack/spack/database.py
@@ -33,7 +33,7 @@ The database serves two purposes:
   2. It will allow us to track external installations as well as lost
      packages and their dependencies.
 
-Prior ot the implementation of this store, a direcotry layout served
+Prior to the implementation of this store, a directory layout served
 as the authoritative database of packages in Spack.  This module
 provides a cache and a sanity checking mechanism for what is in the
 filesystem.
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index aa874bf508..f1c4e22053 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -39,8 +39,17 @@ import re
 import textwrap
 import time
 import string
+import contextlib
+from urlparse import urlparse
+from StringIO import StringIO
 
+import llnl.util.lock
 import llnl.util.tty as tty
+from llnl.util.filesystem import *
+from llnl.util.lang import *
+from llnl.util.link_tree import LinkTree
+from llnl.util.tty.log import log_output
+
 import spack
 import spack.build_environment
 import spack.compilers
@@ -53,11 +62,6 @@ import spack.repository
 import spack.url
 import spack.util.web
 
-from StringIO import StringIO
-from llnl.util.filesystem import *
-from llnl.util.lang import *
-from llnl.util.link_tree import LinkTree
-from llnl.util.tty.log import log_output
 from spack.stage import Stage, ResourceStage, StageComposite
 from spack.util.environment import dump_environment
 from spack.util.executable import ProcessError, which
@@ -346,6 +350,9 @@ class Package(object):
         # this determines how the package should be built.
         self.spec = spec
 
+        # Lock on the prefix shared resource. Will be set in prefix property
+        self._prefix_lock = None
+
         # Name of package is the name of its module, without the
         # containing module names.
         self.name = self.module.__name__
@@ -692,6 +699,22 @@ class Package(object):
         return dependents
 
     @property
+    def prefix_lock(self):
+        if self._prefix_lock is None:
+            dirname = join_path(os.path.dirname(self.spec.prefix), '.locks')
+            basename = os.path.basename(self.spec.prefix)
+            lock_file = join_path(dirname, basename)
+
+            if not os.path.exists(lock_file):
+                tty.debug('TOUCH FILE : {0}'.format(lock_file))
+                os.makedirs(dirname)
+                touch(lock_file)
+
+            self._prefix_lock = llnl.util.lock.Lock(lock_file)
+
+        return self._prefix_lock
+
+    @property
     def prefix(self):
         """Get the prefix into which this package should be installed."""
         return self.spec.prefix
@@ -875,6 +898,22 @@ class Package(object):
         resource_stage_folder = '-'.join(pieces)
         return resource_stage_folder
 
+    @contextlib.contextmanager
+    def _prefix_read_lock(self):
+        try:
+            self.prefix_lock.acquire_read(60)
+            yield self
+        finally:
+            self.prefix_lock.release_read()
+
+    @contextlib.contextmanager
+    def _prefix_write_lock(self):
+        try:
+            self.prefix_lock.acquire_write(60)
+            yield self
+        finally:
+            self.prefix_lock.release_write()
+
     install_phases = set(['configure', 'build', 'install', 'provenance'])
 
     def do_install(self,
@@ -926,14 +965,18 @@ class Package(object):
 
         # Ensure package is not already installed
         layout = spack.install_layout
-        if 'install' in install_phases and layout.check_installed(self.spec):
-            tty.msg("%s is already installed in %s" % (self.name, self.prefix))
-            rec = spack.installed_db.get_record(self.spec)
-            if (not rec.explicit) and explicit:
-                with spack.installed_db.write_transaction():
-                    rec = spack.installed_db.get_record(self.spec)
-                    rec.explicit = True
-            return
+        with self._prefix_read_lock():
+            if ('install' in install_phases and
+                layout.check_installed(self.spec)):
+
+                tty.msg(
+                    "%s is already installed in %s" % (self.name, self.prefix))
+                rec = spack.installed_db.get_record(self.spec)
+                if (not rec.explicit) and explicit:
+                    with spack.installed_db.write_transaction():
+                        rec = spack.installed_db.get_record(self.spec)
+                        rec.explicit = True
+                return
 
         tty.msg("Installing %s" % self.name)
 
@@ -983,7 +1026,7 @@ class Package(object):
             self.build_directory = join_path(self.stage.path, 'spack-build')
             self.source_directory = self.stage.source_path
 
-            with self.stage:
+            with contextlib.nested(self.stage, self._prefix_write_lock()):
                 # Run the pre-install hook in the child process after
                 # the directory is created.
                 spack.hooks.pre_install(self)
@@ -1077,8 +1120,9 @@ class Package(object):
                          wrap=False)
             raise
 
-        # note: PARENT of the build process adds the new package to
+        # Parent of the build process adds the new package to
         # the database, so that we don't need to re-read from file.
+        # NOTE: add() implicitly acquires a write-lock
         spack.installed_db.add(
             self.spec, spack.install_layout, explicit=explicit)
 
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 1b12966bc1..3887af4dad 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -29,6 +29,7 @@ import tempfile
 from urlparse import urljoin
 
 import llnl.util.tty as tty
+import llnl.util.lock
 from llnl.util.filesystem import *
 
 import spack.util.pattern as pattern
@@ -88,8 +89,9 @@ class Stage(object):
     similar, and are intended to persist for only one run of spack.
     """
 
-    def __init__(self, url_or_fetch_strategy,
-                 name=None, mirror_path=None, keep=False, path=None):
+    def __init__(
+            self, url_or_fetch_strategy,
+            name=None, mirror_path=None, keep=False, path=None, lock=True):
         """Create a stage object.
            Parameters:
              url_or_fetch_strategy
@@ -147,6 +149,15 @@ class Stage(object):
         # Flag to decide whether to delete the stage folder on exit or not
         self.keep = keep
 
+        # File lock for the stage directory
+        self._lock_file = None
+        self._lock = None
+        if lock:
+            self._lock_file = join_path(spack.stage_path, self.name + '.lock')
+            if not os.path.exists(self._lock_file):
+                touch(self._lock_file)
+            self._lock = llnl.util.lock.Lock(self._lock_file)
+
     def __enter__(self):
         """
         Entering a stage context will create the stage directory
@@ -154,6 +165,8 @@ class Stage(object):
         Returns:
             self
         """
+        if self._lock is not None:
+            self._lock.acquire_write(timeout=60)
         self.create()
         return self
 
@@ -175,6 +188,9 @@ class Stage(object):
         if exc_type is None and not self.keep:
             self.destroy()
 
+        if self._lock is not None:
+            self._lock.release_write()
+
     def _need_to_create_path(self):
         """Makes sure nothing weird has happened since the last time we
            looked at path.  Returns True if path already exists and is ok.