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,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 os
 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,10 +107,7 @@ class State(object):
         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)
-                        json.dump(self._state, fp, indent=4)
-                    else:
-                        json.dump(self._state, fp)
             except IOError:
                 raise exception.HomeDirPermissionsError(util.get_home_dir())
         self._unlock_state_file()
@@ -119,21 +115,19 @@ class State(object):
     def _lock_state_file(self):
         if not self.lock:
             return
-        self._lockfile = LockFile(self.path)
+        self._lockfile = lockfile.LockFile(self.path)
-
-        if self._lockfile.is_locked() and \
-                (time() - getmtime(self._lockfile.lock_file)) > 10:
-            self._lockfile.break_lock()
-
         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 @@ class ContentCache(object):
     def _lock_dbindex(self):
         if not self.cache_dir:
             os.makedirs(self.cache_dir)
-        self._lockfile = LockFile(self.cache_dir)
+        self._lockfile = 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()
-
         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,108 @@
+# 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.
+
+from os import remove
+from os.path import abspath, exists, getmtime
+from time import sleep, time
+
+from platformio import exception
+
+LOCKFILE_TIMEOUT = 3600  # in seconds, 1 hour
+LOCKFILE_DELAY = 0.2
+
+LOCKFILE_INTERFACE_FCNTL = 1
+LOCKFILE_INTERFACE_MSVCRT = 2
+
+try:
+    import fcntl
+    LOCKFILE_CURRENT_INTERFACE = LOCKFILE_INTERFACE_FCNTL
+except ImportError:
+    try:
+        import msvcrt
+        LOCKFILE_CURRENT_INTERFACE = LOCKFILE_INTERFACE_MSVCRT
+    except ImportError:
+        LOCKFILE_CURRENT_INTERFACE = None
+
+
+class LockFileExists(Exception):
+    pass
+
+
+class LockFile(object):
+
+    def __init__(self, path, timeout=LOCKFILE_TIMEOUT, delay=LOCKFILE_DELAY):
+        self.timeout = timeout
+        self.delay = delay
+        self._lock_path = abspath(path) + ".lock"
+        self._fp = None
+
+    def _lock(self):
+        if not LOCKFILE_CURRENT_INTERFACE and exists(self._lock_path):
+            # remove stale lock
+            if time() - getmtime(self._lock_path) > 10:
+                try:
+                    remove(self._lock_path)
+                except:  # pylint: disable=bare-except
+                    pass
+            else:
+                raise LockFileExists
+
+        self._fp = open(self._lock_path, "w")
+        try:
+            if LOCKFILE_CURRENT_INTERFACE == LOCKFILE_INTERFACE_FCNTL:
+                fcntl.flock(self._fp.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
+            elif LOCKFILE_CURRENT_INTERFACE == LOCKFILE_INTERFACE_MSVCRT:
+                msvcrt.locking(self._fp.fileno(), msvcrt.LK_NBLCK, 1)
+        except IOError:
+            self._fp = None
+            raise LockFileExists
+        return True
+
+    def _unlock(self):
+        if not self._fp:
+            return
+        if LOCKFILE_CURRENT_INTERFACE == LOCKFILE_INTERFACE_FCNTL:
+            fcntl.flock(self._fp.fileno(), fcntl.LOCK_UN)
+        elif LOCKFILE_CURRENT_INTERFACE == LOCKFILE_INTERFACE_MSVCRT:
+            msvcrt.locking(self._fp.fileno(), msvcrt.LK_UNLCK, 1)
+        self._fp.close()
+        self._fp = None
+
+    def acquire(self):
+        elapsed = 0
+        while elapsed < self.timeout:
+            try:
+                return self._lock()
+            except LockFileExists:
+                sleep(self.delay)
+                elapsed += self.delay
+
+        raise exception.LockFileTimeoutError()
+
+    def release(self):
+        self._unlock()
+        if exists(self._lock_path):
+            try:
+                remove(self._lock_path)
+            except:  # pylint: disable=bare-except
+                pass
+
+    def __enter__(self):
+        self.acquire()
+
+    def __exit__(self, type_, value, traceback):
+        self.release()
+
+    def __del__(self):
+        self.release()

platformio/managers/package.py

@@ -27,6 +27,7 @@ import semantic_version
 
 from platformio import __version__, app, exception, telemetry, util
 from platformio.downloader import FileDownloader
+from platformio.lockfile import LockFile
 from platformio.unpacker import FileUnpacker
 from platformio.vcsclient import VCSClientFactory
 
@@ -707,71 +708,82 @@ class BasePkgManager(PkgRepoMixin, PkgInstallerMixin):
                     fg="yellow")
             return package_dir
 
-        if url:
+        pkg_dir = None
-            pkg_dir = self._install_from_url(
+        # interprocess lock
-                name, url, requirements, track=True)
+        with LockFile(self.package_dir):
-        else:
+            self.cache_reset()
-            pkg_dir = self._install_from_piorepo(name, requirements)
 
-        if not pkg_dir or not self.manifest_exists(pkg_dir):
+            if url:
-            raise exception.PackageInstallError(name, requirements or "*",
+                pkg_dir = self._install_from_url(
-                                                util.get_systype())
+                    name, url, requirements, track=True)
+            else:
+                pkg_dir = self._install_from_piorepo(name, requirements)
 
-        manifest = self.load_manifest(pkg_dir)
+            if not pkg_dir or not self.manifest_exists(pkg_dir):
-        assert manifest
+                raise exception.PackageInstallError(name, requirements or "*",
+                                                    util.get_systype())
 
-        if not after_update:
+            manifest = self.load_manifest(pkg_dir)
-            telemetry.on_event(
+            assert manifest
-                category=self.__class__.__name__,
-                action="Install",
-                label=manifest['name'])
 
-        if not silent:
+            if not after_update:
-            click.secho(
+                telemetry.on_event(
-                "{name} @ {version} has been successfully installed!".format(
+                    category=self.__class__.__name__,
-                    **manifest),
+                    action="Install",
-                fg="green")
+                    label=manifest['name'])
+
+            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):
+        # interprocess lock
-            pkg_dir = package
+        with LockFile(self.package_dir):
-        else:
-            name, requirements, url = self.parse_pkg_uri(package, requirements)
-            pkg_dir = self.get_package_dir(name, requirements, url)
-
-        if not pkg_dir:
-            raise exception.UnknownPackage(
-                "%s @ %s" % (package, requirements or "*"))
-
-        manifest = self.load_manifest(pkg_dir)
-        click.echo(
-            "Uninstalling %s @ %s: \t" % (click.style(
-                manifest['name'], fg="cyan"), manifest['version']),
-            nl=False)
-
-        if islink(pkg_dir):
-            os.unlink(pkg_dir)
-        else:
-            util.rmtree_(pkg_dir)
-        self.cache_reset()
-
-        # 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 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)
+
+            if not pkg_dir:
+                raise exception.UnknownPackage(
+                    "%s @ %s" % (package, requirements or "*"))
+
+            manifest = self.load_manifest(pkg_dir)
+            click.echo(
+                "Uninstalling %s @ %s: \t" % (click.style(
+                    manifest['name'], fg="cyan"), manifest['version']),
+                nl=False)
+
+            if islink(pkg_dir):
+                os.unlink(pkg_dir)
+            else:
+                util.rmtree_(pkg_dir)
+            self.cache_reset()
+
+            # 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,6 @@ install_requires = [
     "bottle<0.13",
     "click>=5,<6",
     "colorama",
-    "lockfile>=0.9.1,<0.13",
     "pyserial>=3,<4,!=3.3",
     "requests>=2.4.0,<3",
     "semantic_version>=2.5.0,<3"