From e1578dabac936a1595ff127c07b113804e3d7212 Mon Sep 17 00:00:00 2001 From: Ivan Kravets Date: Sat, 14 Jul 2018 22:10:56 +0300 Subject: [PATCH] Lock interprocess requests to PlatformIO Package Manager for install/uninstall operations // Resolve #1462 --- .isort.cfg | 2 +- HISTORY.rst | 3 + platformio/app.py | 31 +++------ platformio/exception.py | 4 ++ platformio/lockfile.py | 108 +++++++++++++++++++++++++++++ platformio/managers/package.py | 120 ++++++++++++++++++--------------- setup.py | 1 - 7 files changed, 192 insertions(+), 77 deletions(-) create mode 100644 platformio/lockfile.py diff --git a/.isort.cfg b/.isort.cfg index d094cc19..9b58f629 100644 --- a/.isort.cfg +++ b/.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 diff --git a/HISTORY.rst b/HISTORY.rst index e567092f..df3add2a 100644 --- a/HISTORY.rst +++ b/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 `_) +* Lock interprocess requests to PlatformIO Package Manager for + install/uninstall operations + (`issue #1594 `_) 3.5.4 (2018-07-03) ~~~~~~~~~~~~~~~~~~ diff --git a/platformio/app.py b/platformio/app.py index 06ca7cfb..6ace78ed 100644 --- a/platformio/app.py +++ b/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, 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() @@ -119,21 +115,19 @@ class State(object): 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 @@ class ContentCache(object): 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 diff --git a/platformio/exception.py b/platformio/exception.py index c7d3cd2f..8a1b43fc 100644 --- a/platformio/exception.py +++ b/platformio/exception.py @@ -28,6 +28,10 @@ class ReturnErrorCode(PlatformioException): MESSAGE = "{0}" +class LockFileTimeoutError(PlatformioException): + pass + + class MinitermException(PlatformioException): pass diff --git a/platformio/lockfile.py b/platformio/lockfile.py new file mode 100644 index 00000000..f00c16e5 --- /dev/null +++ b/platformio/lockfile.py @@ -0,0 +1,108 @@ +# Copyright (c) 2014-present PlatformIO +# +# 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() diff --git a/platformio/managers/package.py b/platformio/managers/package.py index 3e5bd4a0..686c718b 100644 --- a/platformio/managers/package.py +++ b/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 = self._install_from_url( - name, url, requirements, track=True) - else: - pkg_dir = self._install_from_piorepo(name, requirements) + pkg_dir = None + # interprocess lock + with LockFile(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()) + if url: + pkg_dir = self._install_from_url( + name, url, requirements, track=True) + else: + pkg_dir = self._install_from_piorepo(name, requirements) - manifest = self.load_manifest(pkg_dir) - assert manifest + if not pkg_dir or not self.manifest_exists(pkg_dir): + raise exception.PackageInstallError(name, requirements or "*", + util.get_systype()) - if not after_update: - telemetry.on_event( - category=self.__class__.__name__, - action="Install", - label=manifest['name']) + manifest = self.load_manifest(pkg_dir) + assert manifest - if not silent: - click.secho( - "{name} @ {version} has been successfully installed!".format( - **manifest), - fg="green") + 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") 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) - - 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))) + # interprocess lock + with LockFile(self.package_dir): 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): diff --git a/setup.py b/setup.py index b4b8b1aa..4159e183 100644 --- a/setup.py +++ b/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"