From 9fc05efed8744d1e782e5ef1e3b7a51966dc68f0 Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Thu, 18 Nov 2021 15:32:09 +0800 Subject: [PATCH 1/3] Use optimistic locking for service principal entry reads --- .../azure/cli/core/auth/persistence.py | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/persistence.py b/src/azure-cli-core/azure/cli/core/auth/persistence.py index 6ebec24883f..5fb9a7bebe3 100644 --- a/src/azure-cli-core/azure/cli/core/auth/persistence.py +++ b/src/azure-cli-core/azure/cli/core/auth/persistence.py @@ -8,6 +8,7 @@ import json import sys +import time from msal_extensions import (FilePersistenceWithDataProtection, KeychainPersistence, LibsecretPersistence, FilePersistence, PersistedTokenCache, CrossPlatLock) @@ -60,13 +61,21 @@ def save(self, content): with CrossPlatLock(self._lock_file): self._persistence.save(json.dumps(content, indent=4)) + def _load(self): + try: + return json.loads(self._persistence.load()) + except PersistenceNotFound: + return [] + def load(self): - with CrossPlatLock(self._lock_file): + # Use optimistic locking rather than CrossPlatLock, so that multiple processes can + # read the same file at the same time. + retry = 3 + for attempt in range(1, retry + 1): try: - return json.loads(self._persistence.load()) - except PersistenceNotFound: - return [] - except Exception as ex: - raise CLIError("Failed to load token files. If you can reproduce, please log an issue at " - "https://github.com/Azure/azure-cli/issues. At the same time, you can clean " - "up by running 'az account clear' and then 'az login'. (Inner Error: {})".format(ex)) + return self._load() + except Exception: # pylint: disable=broad-except + # Presumably other processes are writing the file, causing dirty read + logger.debug(f"Unable to load token cache file in No. {attempt} attempt") + time.sleep(0.5) + raise RuntimeError(f"Unable to load token cache file in {retry} attempts") From c9a48e20f27fefe4cda6c56256bf5f997ee32136 Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Thu, 18 Nov 2021 17:07:19 +0800 Subject: [PATCH 2/3] linter --- src/azure-cli-core/azure/cli/core/auth/persistence.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/persistence.py b/src/azure-cli-core/azure/cli/core/auth/persistence.py index 5fb9a7bebe3..bf43c2b52a9 100644 --- a/src/azure-cli-core/azure/cli/core/auth/persistence.py +++ b/src/azure-cli-core/azure/cli/core/auth/persistence.py @@ -14,7 +14,6 @@ FilePersistence, PersistedTokenCache, CrossPlatLock) from msal_extensions.persistence import PersistenceNotFound -from knack.util import CLIError from knack.log import get_logger logger = get_logger(__name__) @@ -76,6 +75,6 @@ def load(self): return self._load() except Exception: # pylint: disable=broad-except # Presumably other processes are writing the file, causing dirty read - logger.debug(f"Unable to load token cache file in No. {attempt} attempt") + logger.debug("Unable to load token cache file in No. %d attempt", attempt) time.sleep(0.5) raise RuntimeError(f"Unable to load token cache file in {retry} attempts") From fc955e7b6a7d5670b49205cd50f9fe9b53aef4f4 Mon Sep 17 00:00:00 2001 From: jiasli <4003950+jiasli@users.noreply.github.com> Date: Mon, 6 Dec 2021 16:34:28 +0800 Subject: [PATCH 3/3] raise original error --- .../azure/cli/core/auth/persistence.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/auth/persistence.py b/src/azure-cli-core/azure/cli/core/auth/persistence.py index bf43c2b52a9..c22a2124f97 100644 --- a/src/azure-cli-core/azure/cli/core/auth/persistence.py +++ b/src/azure-cli-core/azure/cli/core/auth/persistence.py @@ -75,6 +75,12 @@ def load(self): return self._load() except Exception: # pylint: disable=broad-except # Presumably other processes are writing the file, causing dirty read - logger.debug("Unable to load token cache file in No. %d attempt", attempt) - time.sleep(0.5) - raise RuntimeError(f"Unable to load token cache file in {retry} attempts") + if attempt < retry: + logger.debug("Unable to load secret store in No. %d attempt", attempt) + import traceback + logger.debug(traceback.format_exc()) + time.sleep(0.5) + else: + raise # End of retry. Re-raise the exception as-is. + + return [] # Not really reachable here. Just to keep pylint happy.