From 2c8ccf0726fe35214c9983c6f628995bb3194a24 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Wed, 23 May 2018 10:02:34 +0100 Subject: [PATCH 01/18] Add thread-safety locking for the cache feature --- mbed/mbed.py | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 79460f35..f162e9a5 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -42,7 +42,7 @@ import stat import errno import ctypes -from itertools import chain, repeat +import time import zipfile import argparse @@ -1223,7 +1223,9 @@ def clone(self, url, path, rev=None, depth=None, protocol=None, offline=False, * os.makedirs(os.path.split(path)[0]) info("Carbon copy from \"%s\" to \"%s\"" % (cache, path)) + self.cache_lock(url) shutil.copytree(cache, path) + self.cache_unlock(url) with cd(path): scm.seturl(formaturl(url, protocol)) @@ -1253,7 +1255,9 @@ def clone(self, url, path, rev=None, depth=None, protocol=None, offline=False, * self.url = url self.path = os.path.abspath(path) self.ignores() + self.cache_lock(url) self.set_cache(url) + self.cache_unlock(url) return True if offline: @@ -1336,6 +1340,70 @@ def set_cache(self, url): warning("Unable to cache \"%s\" to \"%s\"" % (self.path, cpath)) return False + def cache_lock(self, url): + cpath = self.url2cachedir(url) + if cpath: + lock_file = os.path.join(cpath, '.lock') + try: + if not os.path.isdir(cpath): + os.makedirs(cpath) + + can_lock = False + # this loop is handling a lock file from another process if exists + for i in range(300): + if os.path.exists(lock_file): + # lock file exists, but we need to check pid as well in case the process died + with open(lock_file) as f: + pid = f.read(200) + if pid and int(pid) != os.getpid(): + if self.pid_exists(pid): + time.sleep(1) + continue + else: + info("Cache lock file exists, but process is dead. Cleaning up") + os.unlink(lock_file) + can_lock = True + break + + if can_lock: + with open(lock_file, 'wb') as f: + f.write(str(os.getpid())) + else: + error("Exceeded 5 minutes limit while waiting for other process to finish caching") + except Exception: + error("Unable to lock cache dir \"%s\"" % (cpath)) + return False + + def cache_unlock(self, url): + cpath = self.url2cachedir(url) + if cpath: + lock_file = os.path.join(cpath, '.lock') + if os.path.isfile(os.path.join(cpath, '.lock')): + try: + with open(lock_file) as f: + pid = f.read(200) + if int(pid) == os.getpid(): + os.unlink(os.path.join(cpath, '.lock')) + else: + error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) + except Exception: + error("Unable to unlock cache dir \"%s\"" % (cpath)) + return True + return False + + def pid_exists(self, pid): + try: + os.kill(int(pid), 0) + except OSError as err: + if err.errno == errno.ESRCH: + return False + elif err.errno == errno.EPERM: + return True + else: + raise err + else: + return True + def can_update(self, clean, clean_deps): err = None if (self.is_local or self.url is None) and not clean_deps: From e72ef630d3b34eef158a003ea6de12e334ae5108 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Wed, 6 Jun 2018 14:25:38 +0100 Subject: [PATCH 02/18] Add filesystem error handling during cache_lock() --- mbed/mbed.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index f162e9a5..09724cf2 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1351,17 +1351,24 @@ def cache_lock(self, url): can_lock = False # this loop is handling a lock file from another process if exists for i in range(300): - if os.path.exists(lock_file): - # lock file exists, but we need to check pid as well in case the process died - with open(lock_file) as f: - pid = f.read(200) - if pid and int(pid) != os.getpid(): - if self.pid_exists(pid): - time.sleep(1) + if i: + time.sleep(1) + if os.path.isfile(lock_file): + try: + # lock file exists, but we need to check pid as well in case the process died + with open(lock_file, 'r') as f: + pid = f.read(8) + if pid and int(pid) != os.getpid(): + if self.pid_exists(pid): + continue + else: + info("Cache lock file exists, but process is dead. Cleaning up") + os.remove(lock_file) continue - else: - info("Cache lock file exists, but process is dead. Cleaning up") - os.unlink(lock_file) + except (IOError, OSError): + continue + pass + can_lock = True break @@ -1378,12 +1385,12 @@ def cache_unlock(self, url): cpath = self.url2cachedir(url) if cpath: lock_file = os.path.join(cpath, '.lock') - if os.path.isfile(os.path.join(cpath, '.lock')): + if os.path.isfile(lock_file): try: with open(lock_file) as f: - pid = f.read(200) + pid = f.read(8) if int(pid) == os.getpid(): - os.unlink(os.path.join(cpath, '.lock')) + os.remove(lock_file) else: error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) except Exception: From b1d458fa6b8c05da2899b01543a4a978ce0f5b16 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Thu, 7 Jun 2018 13:01:17 +0100 Subject: [PATCH 03/18] Add no buffering and file flush() to cache_lock() --- mbed/mbed.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 09724cf2..879375ba 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1356,7 +1356,7 @@ def cache_lock(self, url): if os.path.isfile(lock_file): try: # lock file exists, but we need to check pid as well in case the process died - with open(lock_file, 'r') as f: + with open(lock_file, 'r', 0) as f: pid = f.read(8) if pid and int(pid) != os.getpid(): if self.pid_exists(pid): @@ -1373,8 +1373,10 @@ def cache_lock(self, url): break if can_lock: - with open(lock_file, 'wb') as f: + with open(lock_file, 'wb', 0) as f: f.write(str(os.getpid())) + f.flush() + else: error("Exceeded 5 minutes limit while waiting for other process to finish caching") except Exception: @@ -1387,7 +1389,7 @@ def cache_unlock(self, url): lock_file = os.path.join(cpath, '.lock') if os.path.isfile(lock_file): try: - with open(lock_file) as f: + with open(lock_file, 'r', 0) as f: pid = f.read(8) if int(pid) == os.getpid(): os.remove(lock_file) From 9cd20efb6361c9becb38a51c3e59a5cfab1b39ca Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Thu, 7 Jun 2018 13:26:42 +0100 Subject: [PATCH 04/18] Restructure code, add more comments and call os.fsync() when locking cache --- mbed/mbed.py | 61 +++++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 879375ba..22df3001 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1344,43 +1344,46 @@ def cache_lock(self, url): cpath = self.url2cachedir(url) if cpath: lock_file = os.path.join(cpath, '.lock') - try: - if not os.path.isdir(cpath): - os.makedirs(cpath) - - can_lock = False - # this loop is handling a lock file from another process if exists - for i in range(300): - if i: - time.sleep(1) - if os.path.isfile(lock_file): - try: - # lock file exists, but we need to check pid as well in case the process died - with open(lock_file, 'r', 0) as f: - pid = f.read(8) - if pid and int(pid) != os.getpid(): - if self.pid_exists(pid): - continue - else: - info("Cache lock file exists, but process is dead. Cleaning up") - os.remove(lock_file) + if not os.path.isdir(cpath): + os.makedirs(cpath) + + can_lock = False + # this loop is handling a lock file from another process if exists + for i in range(300): + if i: + time.sleep(1) + if os.path.isfile(lock_file): + try: + # lock file exists, but we need to check pid as well in case the process died + with open(lock_file, 'r', 0) as f: + pid = f.read(8) + if pid and int(pid) != os.getpid(): + if self.pid_exists(pid): + info("Cache lock file exists and process %s is alive." % pid) continue - except (IOError, OSError): + else: + info("Cache lock file exists, but process is dead. Cleaning up") + os.remove(lock_file) continue - pass + except (IOError, OSError): + continue + pass - can_lock = True - break + can_lock = True + break - if can_lock: + if can_lock: + try: with open(lock_file, 'wb', 0) as f: + info("Writing cache lock file for pid %s" % os.getpid()) f.write(str(os.getpid())) f.flush() + os.fsync(f) + except (IOError, OSError): + error("Unable to lock cache dir \"%s\"" % (cpath)) - else: - error("Exceeded 5 minutes limit while waiting for other process to finish caching") - except Exception: - error("Unable to lock cache dir \"%s\"" % (cpath)) + else: + error("Exceeded 5 minutes limit while waiting for other process to finish caching") return False def cache_unlock(self, url): From f6d883859ac800792e2208de2c946952736ef696 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Thu, 7 Jun 2018 16:26:22 +0100 Subject: [PATCH 05/18] Fixed missing import due to bad merge --- mbed/mbed.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mbed/mbed.py b/mbed/mbed.py index 22df3001..5d1b9617 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -42,6 +42,7 @@ import stat import errno import ctypes +from itertools import chain, repeat import time import zipfile import argparse From c3b671db86cd7ac079662e0e48a73d9fb5e877ed Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Thu, 14 Jun 2018 23:12:13 +0300 Subject: [PATCH 06/18] Refactored implementation --- mbed/mbed.py | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 5d1b9617..f2ac5033 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -45,7 +45,7 @@ from itertools import chain, repeat import time import zipfile -import argparse +from random import randint # Application version @@ -156,7 +156,7 @@ def log(msg, is_error=False): sys.stderr.write(msg) if is_error else sys.stdout.write(msg) def message(msg): - return "[mbed] %s\n" % msg + return "[mbed-%s] %s\n" % (os.getpid(), msg) def info(msg, level=1): if level <= 0 or verbose: @@ -1345,6 +1345,7 @@ def cache_lock(self, url): cpath = self.url2cachedir(url) if cpath: lock_file = os.path.join(cpath, '.lock') + if not os.path.isdir(cpath): os.makedirs(cpath) @@ -1353,35 +1354,29 @@ def cache_lock(self, url): for i in range(300): if i: time.sleep(1) - if os.path.isfile(lock_file): - try: - # lock file exists, but we need to check pid as well in case the process died + + try: + # lock file exists, but we need to check pid as well in case the process died + if os.path.isfile(lock_file): with open(lock_file, 'r', 0) as f: - pid = f.read(8) - if pid and int(pid) != os.getpid(): - if self.pid_exists(pid): - info("Cache lock file exists and process %s is alive." % pid) - continue + pid = int(f.read(8)) + if pid != os.getpid() and self.pid_exists(pid): + info("Cache lock file exists and process %s is alive." % pid) else: - info("Cache lock file exists, but process is dead. Cleaning up") + info("Cache lock file exists, but %s is dead. Cleaning up" % pid) os.remove(lock_file) - continue - except (IOError, OSError): continue - pass - - can_lock = True - break - if can_lock: - try: with open(lock_file, 'wb', 0) as f: - info("Writing cache lock file for pid %s" % os.getpid()) + info("Writing cache lock file %s for pid %s" % (lock_file, os.getpid())) f.write(str(os.getpid())) f.flush() os.fsync(f) + break except (IOError, OSError): - error("Unable to lock cache dir \"%s\"" % (cpath)) + error("OS error occurred") + except Exception as e: + error("Weird error occurred") else: error("Exceeded 5 minutes limit while waiting for other process to finish caching") From 5f86b4eab7c791785b0e72c54b390c8490fd3a56 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Fri, 15 Jun 2018 00:01:36 +0300 Subject: [PATCH 07/18] Make use of folders which are concurrent safe --- mbed/mbed.py | 89 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 37 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index f2ac5033..a6084c6f 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1343,61 +1343,76 @@ def set_cache(self, url): def cache_lock(self, url): cpath = self.url2cachedir(url) - if cpath: - lock_file = os.path.join(cpath, '.lock') + if not cpath: + return False - if not os.path.isdir(cpath): - os.makedirs(cpath) + if not os.path.isdir(cpath): + os.makedirs(cpath) - can_lock = False - # this loop is handling a lock file from another process if exists - for i in range(300): - if i: - time.sleep(1) + lock_dir = os.path.join(cpath, '.lock') + lock_file = os.path.join(lock_dir, 'pid') - try: - # lock file exists, but we need to check pid as well in case the process died - if os.path.isfile(lock_file): - with open(lock_file, 'r', 0) as f: - pid = int(f.read(8)) - if pid != os.getpid() and self.pid_exists(pid): - info("Cache lock file exists and process %s is alive." % pid) - else: - info("Cache lock file exists, but %s is dead. Cleaning up" % pid) - os.remove(lock_file) - continue + # this loop is handling a lock file from another process if exists + for i in range(300): + if i: + time.sleep(1) + # lock file exists, but we need to check pid as well in case the process died + if os.path.exists(lock_dir): + if os.path.isfile(lock_file): + with open(lock_file, 'r', 0) as f: + pid = int(f.read(8)) + if pid != os.getpid() and self.pid_exists(pid): + info("Cache lock file exists and process %s is alive." % pid) + else: + info("Cache lock file exists, but %s is dead. Cleaning up" % pid) + os.remove(lock_file) + os.rmdir(lock_dir) + else: + os.rmdir(lock_dir) + continue + else: + try: + os.mkdir(lock_dir) with open(lock_file, 'wb', 0) as f: info("Writing cache lock file %s for pid %s" % (lock_file, os.getpid())) f.write(str(os.getpid())) f.flush() os.fsync(f) - break - except (IOError, OSError): - error("OS error occurred") - except Exception as e: - error("Weird error occurred") - - else: - error("Exceeded 5 minutes limit while waiting for other process to finish caching") - return False + break + except (OSError, WindowsError) as e: + ## Windows: + ## 17 [Error 183] Cannot create a file when that file already exists: 'testing' + ## or when concurrent: 13 WindowsError(5, 'Access is denied') + ## Linux: 17 [Errno 17] File exists: 'testing' + ## or when concurrent & virtualbox 71, OSError(71, 'Protocol error') + ## or when full: 28, OSError(28, 'No space left on device') + if e.errno in (17,13,71,28): + continue + else: + raise e + else: + error("Exceeded 5 minutes limit while waiting for other process to finish caching") + return True def cache_unlock(self, url): cpath = self.url2cachedir(url) - if cpath: - lock_file = os.path.join(cpath, '.lock') + if not cpath: + return False + lock_dir = os.path.join(cpath, '.lock') + lock_file = os.path.join(lock_dir, 'pid') + if os.path.exists(lock_dir): if os.path.isfile(lock_file): try: with open(lock_file, 'r', 0) as f: pid = f.read(8) - if int(pid) == os.getpid(): - os.remove(lock_file) - else: + if int(pid) != os.getpid(): error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) - except Exception: + except OSError: error("Unable to unlock cache dir \"%s\"" % (cpath)) - return True - return False + os.remove(lock_file) + os.rmdir(lock_dir) + return True def pid_exists(self, pid): try: From ea97bc6cb5ac7b46527a190e81389670bf08cec8 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Fri, 15 Jun 2018 00:05:08 +0300 Subject: [PATCH 08/18] Remove obsolete comments --- mbed/mbed.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index a6084c6f..edb8093a 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1352,12 +1352,10 @@ def cache_lock(self, url): lock_dir = os.path.join(cpath, '.lock') lock_file = os.path.join(lock_dir, 'pid') - # this loop is handling a lock file from another process if exists for i in range(300): if i: time.sleep(1) - # lock file exists, but we need to check pid as well in case the process died if os.path.exists(lock_dir): if os.path.isfile(lock_file): with open(lock_file, 'r', 0) as f: @@ -1399,6 +1397,7 @@ def cache_unlock(self, url): cpath = self.url2cachedir(url) if not cpath: return False + lock_dir = os.path.join(cpath, '.lock') lock_file = os.path.join(lock_dir, 'pid') if os.path.exists(lock_dir): From fb7fa822701f7fea370364206849bfd4c265c648 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Mon, 18 Jun 2018 14:56:04 +0100 Subject: [PATCH 09/18] Remove WindowsError for now --- mbed/mbed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index edb8093a..c892e763 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1378,7 +1378,7 @@ def cache_lock(self, url): f.flush() os.fsync(f) break - except (OSError, WindowsError) as e: + except (OSError) as e: ## Windows: ## 17 [Error 183] Cannot create a file when that file already exists: 'testing' ## or when concurrent: 13 WindowsError(5, 'Access is denied') From f1337aa2ed341bf10f5de8a110ab36216932040f Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Mon, 18 Jun 2018 20:28:35 +0100 Subject: [PATCH 10/18] Handle case where pid file is in the middle of write --- mbed/mbed.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index c892e763..6b926e7a 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1351,16 +1351,22 @@ def cache_lock(self, url): lock_dir = os.path.join(cpath, '.lock') lock_file = os.path.join(lock_dir, 'pid') + timeout = 300 - for i in range(300): + for i in range(timeout): if i: time.sleep(1) if os.path.exists(lock_dir): if os.path.isfile(lock_file): with open(lock_file, 'r', 0) as f: - pid = int(f.read(8)) - if pid != os.getpid() and self.pid_exists(pid): + pid = f.read(8) + if not pid: + if int(os.path.getmtime(lock_file)) + timeout < int(time.time()): + info("Cache lock file exists, but is empty. Cleaning up") + os.remove(lock_file) + os.rmdir(lock_dir) + elif int(pid) != os.getpid() and self.pid_exists(pid): info("Cache lock file exists and process %s is alive." % pid) else: info("Cache lock file exists, but %s is dead. Cleaning up" % pid) From cfd09c87567091cfa5308ad8e17c4336d6dd6e2f Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Thu, 21 Jun 2018 17:32:02 +0100 Subject: [PATCH 11/18] Handle empty dir issue --- mbed/mbed.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 6b926e7a..bb57a29a 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1358,23 +1358,26 @@ def cache_lock(self, url): time.sleep(1) if os.path.exists(lock_dir): - if os.path.isfile(lock_file): - with open(lock_file, 'r', 0) as f: - pid = f.read(8) - if not pid: - if int(os.path.getmtime(lock_file)) + timeout < int(time.time()): - info("Cache lock file exists, but is empty. Cleaning up") + try: + if os.path.isfile(lock_file): + with open(lock_file, 'r', 0) as f: + pid = f.read(8) + if not pid: + if int(os.path.getmtime(lock_file)) + timeout < int(time.time()): + info("Cache lock file exists, but is empty. Cleaning up") + os.remove(lock_file) + os.rmdir(lock_dir) + elif int(pid) != os.getpid() and self.pid_exists(pid): + info("Cache lock file exists and process %s is alive." % pid) + else: + info("Cache lock file exists, but %s is dead. Cleaning up" % pid) os.remove(lock_file) os.rmdir(lock_dir) - elif int(pid) != os.getpid() and self.pid_exists(pid): - info("Cache lock file exists and process %s is alive." % pid) else: - info("Cache lock file exists, but %s is dead. Cleaning up" % pid) - os.remove(lock_file) os.rmdir(lock_dir) - else: - os.rmdir(lock_dir) - continue + continue + except (OSError) as e: + continue else: try: os.mkdir(lock_dir) From 0a2c439370ef13103b3d23d7e1d8472ef604fa09 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Tue, 26 Jun 2018 11:50:53 +0100 Subject: [PATCH 12/18] Ignore errors while releasing cache --- mbed/mbed.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index bb57a29a..b12c00a3 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1409,17 +1409,20 @@ def cache_unlock(self, url): lock_dir = os.path.join(cpath, '.lock') lock_file = os.path.join(lock_dir, 'pid') - if os.path.exists(lock_dir): - if os.path.isfile(lock_file): - try: - with open(lock_file, 'r', 0) as f: - pid = f.read(8) - if int(pid) != os.getpid(): - error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) - except OSError: - error("Unable to unlock cache dir \"%s\"" % (cpath)) - os.remove(lock_file) - os.rmdir(lock_dir) + try: + if os.path.exists(lock_dir): + if os.path.isfile(lock_file): + try: + with open(lock_file, 'r', 0) as f: + pid = f.read(8) + if int(pid) != os.getpid(): + error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) + except OSError: + error("Unable to unlock cache dir \"%s\"" % (cpath)) + os.remove(lock_file) + os.rmdir(lock_dir) + except (OSError) as e: + pass return True def pid_exists(self, pid): From a2e0a91960c70c65bfa0ef85a20b51b0661c6d93 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Tue, 26 Jun 2018 11:55:35 +0100 Subject: [PATCH 13/18] Add traceback to some errors --- mbed/mbed.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mbed/mbed.py b/mbed/mbed.py index b12c00a3..55090cd9 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -3329,6 +3329,7 @@ def main(): "You could retry the last command with \"-v\" flag for verbose output\n", e.args[0]) else: error('OS Error: %s' % e.args[1], e.args[0]) + traceback.print_exc(file=sys.stdout) except KeyboardInterrupt: info('User aborted!', -1) sys.exit(255) From 8a70d09f978413a0358c3ede4183122493bc0bab Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Tue, 11 Sep 2018 10:20:15 +0100 Subject: [PATCH 14/18] Fix merge errors --- mbed/mbed.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mbed/mbed.py b/mbed/mbed.py index 55090cd9..2385bb12 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -45,6 +45,7 @@ from itertools import chain, repeat import time import zipfile +import argparse from random import randint From a0fd0d3bd67ba3d014a6bb567b7e909fb677a63a Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Tue, 11 Sep 2018 10:31:06 +0100 Subject: [PATCH 15/18] Remove debug messages --- mbed/mbed.py | 1 - 1 file changed, 1 deletion(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 2385bb12..eda60d15 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -3330,7 +3330,6 @@ def main(): "You could retry the last command with \"-v\" flag for verbose output\n", e.args[0]) else: error('OS Error: %s' % e.args[1], e.args[0]) - traceback.print_exc(file=sys.stdout) except KeyboardInterrupt: info('User aborted!', -1) sys.exit(255) From 13ab3f2210748669cc379061b1bdfcac2d95ca89 Mon Sep 17 00:00:00 2001 From: Mihail Stoyanov Date: Tue, 11 Sep 2018 19:29:25 +0100 Subject: [PATCH 16/18] Python 3 fixes and also less verbose output --- mbed/mbed.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index eda60d15..14a0e53c 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -157,7 +157,10 @@ def log(msg, is_error=False): sys.stderr.write(msg) if is_error else sys.stdout.write(msg) def message(msg): - return "[mbed-%s] %s\n" % (os.getpid(), msg) + if very_verbose: + return "[mbed-%s] %s\n" % (os.getpid(), msg) + else: + return "[mbed] %s\n" % msg def info(msg, level=1): if level <= 0 or verbose: @@ -1361,7 +1364,7 @@ def cache_lock(self, url): if os.path.exists(lock_dir): try: if os.path.isfile(lock_file): - with open(lock_file, 'r', 0) as f: + with open(lock_file, 'r') as f: pid = f.read(8) if not pid: if int(os.path.getmtime(lock_file)) + timeout < int(time.time()): @@ -1382,7 +1385,7 @@ def cache_lock(self, url): else: try: os.mkdir(lock_dir) - with open(lock_file, 'wb', 0) as f: + with open(lock_file, 'w') as f: info("Writing cache lock file %s for pid %s" % (lock_file, os.getpid())) f.write(str(os.getpid())) f.flush() @@ -1414,7 +1417,7 @@ def cache_unlock(self, url): if os.path.exists(lock_dir): if os.path.isfile(lock_file): try: - with open(lock_file, 'r', 0) as f: + with open(lock_file, 'r') as f: pid = f.read(8) if int(pid) != os.getpid(): error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) From 8a6c2e0eadea3ad0b5ba598e283cd04a7a3f44c3 Mon Sep 17 00:00:00 2001 From: Jimmy Brisson Date: Wed, 12 Sep 2018 09:21:45 -0500 Subject: [PATCH 17/18] Add message for cleaning up the correct behavoir of cache lock --- mbed/mbed.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mbed/mbed.py b/mbed/mbed.py index 14a0e53c..1ffb0050 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -1421,6 +1421,8 @@ def cache_unlock(self, url): pid = f.read(8) if int(pid) != os.getpid(): error("Cache lock file exists with a different pid (\"%s\" vs \"%s\")" % (pid, os.getpid())) + else: + info("Cache lock file exists with my pid (\"%s\"). Cleaning up." % (pid)) except OSError: error("Unable to unlock cache dir \"%s\"" % (cpath)) os.remove(lock_file) From d0c1dd69cad0b911a603680432eaa8143fbb531b Mon Sep 17 00:00:00 2001 From: Jimmy Brisson Date: Wed, 12 Sep 2018 09:23:53 -0500 Subject: [PATCH 18/18] Use a context manager for locking the cache Should help reduce the amount of programmer errors --- mbed/mbed.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/mbed/mbed.py b/mbed/mbed.py index 1ffb0050..a1faa818 100755 --- a/mbed/mbed.py +++ b/mbed/mbed.py @@ -47,6 +47,7 @@ import zipfile import argparse from random import randint +from contextlib import contextmanager # Application version @@ -1228,9 +1229,8 @@ def clone(self, url, path, rev=None, depth=None, protocol=None, offline=False, * os.makedirs(os.path.split(path)[0]) info("Carbon copy from \"%s\" to \"%s\"" % (cache, path)) - self.cache_lock(url) - shutil.copytree(cache, path) - self.cache_unlock(url) + with self.cache_lock_held(url): + shutil.copytree(cache, path) with cd(path): scm.seturl(formaturl(url, protocol)) @@ -1260,9 +1260,8 @@ def clone(self, url, path, rev=None, depth=None, protocol=None, offline=False, * self.url = url self.path = os.path.abspath(path) self.ignores() - self.cache_lock(url) - self.set_cache(url) - self.cache_unlock(url) + with self.cache_lock_held(url): + self.set_cache(url) return True if offline: @@ -1431,6 +1430,14 @@ def cache_unlock(self, url): pass return True + @contextmanager + def cache_lock_held(self, url): + self.cache_lock(url) + try: + yield + finally: + self.cache_unlock(url) + def pid_exists(self, pid): try: os.kill(int(pid), 0)