Lock interprocess requests to PlatformIO Package Manager for install/…

…uninstall operations // Resolve #1462

.isort.cfg

@@ -1,3 +1,3 @@
 [settings]
 line_length=79
-known_third_party=bottle,click,lockfile,python-dateutil,pytest,requests,SCons,semantic_version,serial
+known_third_party=bottle,click,zc.lockfile,pytest,requests,SCons,semantic_version,serial

HISTORY.rst

@@ -18,6 +18,9 @@ PlatformIO 3.0
 * Check package structure after unpacking and raise error when antivirus tool
   blocks PlatformIO package manager
   (`issue #1462 <https://github.com/platformio/platformio-core/issues/1462>`_)
+* Lock interprocess requests to PlatformIO Package Manager for
+  install/uninstall operations
+  (`issue #1594 <https://github.com/platformio/platformio-core/issues/1594>`_)
 
 3.5.4 (2018-07-03)
 ~~~~~~~~~~~~~~~~~~

platformio/app.py

@@ -19,13 +19,12 @@
 import uuid
 from copy import deepcopy
 from os import environ, getenv, listdir, remove
-from os.path import abspath, dirname, expanduser, getmtime, isdir, isfile, join
+from os.path import abspath, dirname, expanduser, isdir, isfile, join
 from time import time
 
 import requests
-from lockfile import LockFailed, LockFile
 
-from platformio import __version__, exception, util
+from platformio import exception, lockfile, util
 
 
 def projects_dir_validate(projects_dir):
@@ -108,32 +107,27 @@ def __exit__(self, type_, value, traceback):
         if self._prev_state != self._state:
             try:
                 with codecs.open(self.path, "w", encoding="utf8") as fp:
-                    if "dev" in __version__:
-                        json.dump(self._state, fp, indent=4)
-                    else:
-                        json.dump(self._state, fp)
+                    json.dump(self._state, fp)
             except IOError:
                 raise exception.HomeDirPermissionsError(util.get_home_dir())
         self._unlock_state_file()
 
     def _lock_state_file(self):
         if not self.lock:
             return
-        self._lockfile = LockFile(self.path)
-
-        if self._lockfile.is_locked() and \
-                (time() - getmtime(self._lockfile.lock_file)) > 10:
-            self._lockfile.break_lock()
-
+        self._lockfile = lockfile.LockFile(self.path)
         try:
             self._lockfile.acquire()
-        except LockFailed:
+        except IOError:
             raise exception.HomeDirPermissionsError(dirname(self.path))
 
     def _unlock_state_file(self):
         if self._lockfile:
             self._lockfile.release()
 
+    def __del__(self):
+        self._unlock_state_file()
+
 
 class ContentCache(object):
 
@@ -155,15 +149,10 @@ def __exit__(self, type_, value, traceback):
     def _lock_dbindex(self):
         if not self.cache_dir:
             os.makedirs(self.cache_dir)
-        self._lockfile = LockFile(self.cache_dir)
-        if self._lockfile.is_locked() and \
-                isfile(self._lockfile.lock_file) and \
-                (time() - getmtime(self._lockfile.lock_file)) > 10:
-            self._lockfile.break_lock()
-
+        self._lockfile = lockfile.LockFile(self.cache_dir)
         try:
             self._lockfile.acquire()
-        except LockFailed:
+        except:  # pylint: disable=bare-except
             return False
 
         return True

platformio/exception.py

@@ -28,6 +28,10 @@ class ReturnErrorCode(PlatformioException):
     MESSAGE = "{0}"
 
 
+class LockFileTimeoutError(PlatformioException):
+    pass
+
+
 class MinitermException(PlatformioException):
     pass
 

platformio/lockfile.py

@@ -0,0 +1,73 @@
+# Copyright (c) 2014-present PlatformIO <contact@platformio.org>
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import logging
+from os import remove
+from os.path import abspath, exists
+from time import sleep
+
+import zc.lockfile
+
+from platformio import exception
+
+logging.getLogger("zc.lockfile").setLevel(logging.ERROR + 1)
+
+LOCKFILE_TIMEOUT = 3600  # in seconds, 1 hour
+LOCKFILE_DELAY = 0.2
+
+
+class LockFile(object):
+
+    def __init__(self, path):
+        self._lock_path = abspath(path) + ".lock"
+        self._lock = None
+
+    def acquire(self, timeout=LOCKFILE_TIMEOUT, delay=LOCKFILE_DELAY):
+        elapsed = 0
+        while elapsed < timeout:
+            try:
+                self._lock = zc.lockfile.LockFile(self._lock_path)
+                return
+            except zc.lockfile.LockError:
+                sleep(delay)
+                elapsed += delay
+
+        raise exception.LockFileTimeoutError()
+
+    def release(self):
+        if self._lock:
+            self._lock.close()
+            self._lock = None
+        if exists(self._lock_path):
+            try:
+                remove(self._lock_path)
+            except:  # pylint: disable=bare-except
+                pass
+
+
+class LockFileContext(object):
+
+    def __init__(self, path, timeout=LOCKFILE_TIMEOUT, delay=LOCKFILE_DELAY):
+        self.timeout = timeout
+        self.delay = delay
+        self._lock = LockFile(path)
+
+    def __enter__(self):
+        self._lock.acquire(self.timeout, self.delay)
+
+    def __exit__(self, type_, value, traceback):
+        self._lock.release()
+
+    def __del__(self):
+        self._lock.release()

platformio/managers/package.py

@@ -27,6 +27,7 @@
 
 from platformio import __version__, app, exception, telemetry, util
 from platformio.downloader import FileDownloader
+from platformio.lockfile import LockFileContext
 from platformio.unpacker import FileUnpacker
 from platformio.vcsclient import VCSClientFactory
 
@@ -553,6 +554,7 @@ def __init__(self, package_dir, repositories=None):
         if not isdir(self.package_dir):
             os.makedirs(self.package_dir)
         assert isdir(self.package_dir)
+        self._lockfile = None
 
     @property
     def manifest_names(self):
@@ -679,99 +681,109 @@ def install(self,
                 silent=False,
                 after_update=False,
                 force=False):
-        name, requirements, url = self.parse_pkg_uri(name, requirements)
-        package_dir = self.get_package_dir(name, requirements, url)
-
-        # avoid circle dependencies
-        if not self.INSTALL_HISTORY:
-            self.INSTALL_HISTORY = []
-        history_key = "%s-%s-%s" % (name, requirements or "", url or "")
-        if history_key in self.INSTALL_HISTORY:
-            return package_dir
-        self.INSTALL_HISTORY.append(history_key)
-
-        if package_dir and force:
-            self.uninstall(package_dir)
-            package_dir = None
-
-        if not package_dir or not silent:
-            msg = "Installing " + click.style(name, fg="cyan")
-            if requirements:
-                msg += " @ " + requirements
-            self.print_message(msg)
-        if package_dir:
-            if not silent:
-                click.secho(
-                    "{name} @ {version} is already installed".format(
-                        **self.load_manifest(package_dir)),
-                    fg="yellow")
-            return package_dir
-
-        if url:
-            pkg_dir = self._install_from_url(
-                name, url, requirements, track=True)
-        else:
-            pkg_dir = self._install_from_piorepo(name, requirements)
+        pkg_dir = None
+        # interprocess lock
+        with LockFileContext(self.package_dir):
+            self.cache_reset()
 
-        if not pkg_dir or not self.manifest_exists(pkg_dir):
-            raise exception.PackageInstallError(name, requirements or "*",
-                                                util.get_systype())
+            name, requirements, url = self.parse_pkg_uri(name, requirements)
+            package_dir = self.get_package_dir(name, requirements, url)
+
+            # avoid circle dependencies
+            if not self.INSTALL_HISTORY:
+                self.INSTALL_HISTORY = []
+            history_key = "%s-%s-%s" % (name, requirements or "", url or "")
+            if history_key in self.INSTALL_HISTORY:
+                return package_dir
+            self.INSTALL_HISTORY.append(history_key)
+
+            if package_dir and force:
+                self.uninstall(package_dir)
+                package_dir = None
+
+            if not package_dir or not silent:
+                msg = "Installing " + click.style(name, fg="cyan")
+                if requirements:
+                    msg += " @ " + requirements
+                self.print_message(msg)
+            if package_dir:
+                if not silent:
+                    click.secho(
+                        "{name} @ {version} is already installed".format(
+                            **self.load_manifest(package_dir)),
+                        fg="yellow")
+                return package_dir
 
-        manifest = self.load_manifest(pkg_dir)
-        assert manifest
+            if url:
+                pkg_dir = self._install_from_url(
+                    name, url, requirements, track=True)
+            else:
+                pkg_dir = self._install_from_piorepo(name, requirements)
 
-        if not after_update:
-            telemetry.on_event(
-                category=self.__class__.__name__,
-                action="Install",
-                label=manifest['name'])
+            if not pkg_dir or not self.manifest_exists(pkg_dir):
+                raise exception.PackageInstallError(name, requirements or "*",
+                                                    util.get_systype())
+
+            manifest = self.load_manifest(pkg_dir)
+            assert manifest
+
+            if not after_update:
+                telemetry.on_event(
+                    category=self.__class__.__name__,
+                    action="Install",
+                    label=manifest['name'])
 
-        if not silent:
-            click.secho(
-                "{name} @ {version} has been successfully installed!".format(
-                    **manifest),
-                fg="green")
+            if not silent:
+                click.secho(
+                    "{name} @ {version} has been successfully installed!".format(
+                        **manifest),
+                    fg="green")
 
         return pkg_dir
 
     def uninstall(self, package, requirements=None, after_update=False):
-        if isdir(package) and self.get_package_by_dir(package):
-            pkg_dir = package
-        else:
-            name, requirements, url = self.parse_pkg_uri(package, requirements)
-            pkg_dir = self.get_package_dir(name, requirements, url)
+        # interprocess lock
+        with LockFileContext(self.package_dir):
+            self.cache_reset()
 
-        if not pkg_dir:
-            raise exception.UnknownPackage(
-                "%s @ %s" % (package, requirements or "*"))
+            if isdir(package) and self.get_package_by_dir(package):
+                pkg_dir = package
+            else:
+                name, requirements, url = self.parse_pkg_uri(package, requirements)
+                pkg_dir = self.get_package_dir(name, requirements, url)
 
-        manifest = self.load_manifest(pkg_dir)
-        click.echo(
-            "Uninstalling %s @ %s: \t" % (click.style(
-                manifest['name'], fg="cyan"), manifest['version']),
-            nl=False)
+            if not pkg_dir:
+                raise exception.UnknownPackage(
+                    "%s @ %s" % (package, requirements or "*"))
 
-        if islink(pkg_dir):
-            os.unlink(pkg_dir)
-        else:
-            util.rmtree_(pkg_dir)
-        self.cache_reset()
+            manifest = self.load_manifest(pkg_dir)
+            click.echo(
+                "Uninstalling %s @ %s: \t" % (click.style(
+                    manifest['name'], fg="cyan"), manifest['version']),
+                nl=False)
 
-        # unfix package with the same name
-        pkg_dir = self.get_package_dir(manifest['name'])
-        if pkg_dir and "@" in pkg_dir:
-            shutil.move(
-                pkg_dir,
-                join(self.package_dir, self.get_install_dirname(manifest)))
+            if islink(pkg_dir):
+                os.unlink(pkg_dir)
+            else:
+                util.rmtree_(pkg_dir)
             self.cache_reset()
 
-        click.echo("[%s]" % click.style("OK", fg="green"))
+            # unfix package with the same name
+            pkg_dir = self.get_package_dir(manifest['name'])
+            if pkg_dir and "@" in pkg_dir:
+                shutil.move(
+                    pkg_dir,
+                    join(self.package_dir, self.get_install_dirname(manifest)))
+                self.cache_reset()
+
+            click.echo("[%s]" % click.style("OK", fg="green"))
+
+            if not after_update:
+                telemetry.on_event(
+                    category=self.__class__.__name__,
+                    action="Uninstall",
+                    label=manifest['name'])
 
-        if not after_update:
-            telemetry.on_event(
-                category=self.__class__.__name__,
-                action="Uninstall",
-                label=manifest['name'])
         return True
 
     def update(self, package, requirements=None, only_check=False):

setup.py

@@ -21,7 +21,7 @@
     "bottle<0.13",
     "click>=5,<6",
     "colorama",
-    "lockfile>=0.9.1,<0.13",
+    "zc.lockfile",
     "pyserial>=3,<4,!=3.3",
     "requests>=2.4.0,<3",
     "semantic_version>=2.5.0,<3"