From d597ee271e1085e10f89fa9632491b3647ed95b1 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Mon, 7 Aug 2017 15:37:37 -0700 Subject: [PATCH] Update globals safely The right way to update these globals is to use a lock and ensure that nobody else is updating them at the same time. Also update a temporary dictionary before setting the global one so that nobody sees partial updates to the global one. This should help fix the thread-safety of shade (and other tooling built ontop of this library). Change-Id: Ie0e0369d98ba6a01edcbf447378a786eec3f13f9 --- os_client_config/constructors.py | 14 +++++++++++--- os_client_config/defaults.py | 21 ++++++++++++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/os_client_config/constructors.py b/os_client_config/constructors.py index e88ac92..579bb2d 100644 --- a/os_client_config/constructors.py +++ b/os_client_config/constructors.py @@ -14,15 +14,23 @@ import json import os +import threading _json_path = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'constructors.json') _class_mapping = None +_class_mapping_lock = threading.Lock() def get_constructor_mapping(): global _class_mapping - if not _class_mapping: + if _class_mapping is not None: + return _class_mapping.copy() + with _class_mapping_lock: + if _class_mapping is not None: + return _class_mapping.copy() + tmp_class_mapping = {} with open(_json_path, 'r') as json_file: - _class_mapping = json.load(json_file) - return _class_mapping + tmp_class_mapping.update(json.load(json_file)) + _class_mapping = tmp_class_mapping + return tmp_class_mapping.copy() diff --git a/os_client_config/defaults.py b/os_client_config/defaults.py index c10358a..1231cce 100644 --- a/os_client_config/defaults.py +++ b/os_client_config/defaults.py @@ -14,19 +14,30 @@ import json import os +import threading _json_path = os.path.join( os.path.dirname(os.path.realpath(__file__)), 'defaults.json') _defaults = None +_defaults_lock = threading.Lock() def get_defaults(): global _defaults - if not _defaults: + if _defaults is not None: + return _defaults.copy() + with _defaults_lock: + if _defaults is not None: + # Did someone else just finish filling it? + return _defaults.copy() # Python language specific defaults # These are defaults related to use of python libraries, they are # not qualities of a cloud. - _defaults = dict( + # + # NOTE(harlowja): update a in-memory dict, before updating + # the global one so that other callers of get_defaults do not + # see the partially filled one. + tmp_defaults = dict( api_timeout=None, verify=True, cacert=None, @@ -36,6 +47,6 @@ def get_defaults(): with open(_json_path, 'r') as json_file: updates = json.load(json_file) if updates is not None: - _defaults.update(updates) - - return _defaults.copy() + tmp_defaults.update(updates) + _defaults = tmp_defaults + return tmp_defaults.copy()