URL: https://github.com/freeipa/freeipa/pull/467 Author: dkupka Title: #467: ipaclient: schema cache: Write all schema files in concurrent-safe way Action: opened
PR body: """ https://fedorahosted.org/freeipa/ticket/6668 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/467/head:pr467 git checkout pr467
From cc9cb78c3e34d946371ccde222560d92a41fa466 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Wed, 15 Feb 2017 09:34:10 +0100 Subject: [PATCH] ipaclient: schema cache: Write all schema files in concurrent-safe way https://fedorahosted.org/freeipa/ticket/6668 --- ipaclient/remote_plugins/__init__.py | 5 ++++- ipaclient/remote_plugins/schema.py | 13 +++---------- ipapython/ipautil.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/ipaclient/remote_plugins/__init__.py b/ipaclient/remote_plugins/__init__.py index da7004d..7b8bdd3 100644 --- a/ipaclient/remote_plugins/__init__.py +++ b/ipaclient/remote_plugins/__init__.py @@ -14,6 +14,8 @@ from ipaclient.plugins.rpcclient import rpcclient from ipapython.dnsutil import DNSName from ipapython.ipa_log_manager import log_mgr +from ipapython.ipautil import concurrent_open + logger = log_mgr.get_logger(__name__) @@ -58,7 +60,8 @@ def _write(self): except EnvironmentError as e: if e.errno != errno.EEXIST: raise - with open(self._path, 'w') as sc: + + with concurrent_open(self._path, 'w') as sc: json.dump(self._dict, sc) except EnvironmentError as e: logger.warning('Failed to write server info: {}'.format(e)) diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py index 15c03f4..2a7cd79 100644 --- a/ipaclient/remote_plugins/schema.py +++ b/ipaclient/remote_plugins/schema.py @@ -25,6 +25,7 @@ from ipapython.dn import DN from ipapython.dnsutil import DNSName from ipapython.ipa_log_manager import log_mgr +from ipapython.ipautil import concurrent_open FORMAT = '1' @@ -407,17 +408,9 @@ def __init__(self, client, fingerprint=None): @contextlib.contextmanager def _open(self, filename, mode): path = os.path.join(self._DIR, filename) + with concurrent_open(path, mode) as f: + yield f - with io.open(path, mode) as f: - if mode.startswith('r'): - fcntl.flock(f, fcntl.LOCK_SH) - else: - fcntl.flock(f, fcntl.LOCK_EX) - - try: - yield f - finally: - fcntl.flock(f, fcntl.LOCK_UN) def _fetch(self, client, ignore_cache=False): if not client.isconnected(): diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index 60b4a37..2724d32 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -282,6 +282,37 @@ def write_tmp_file(txt): return fd + +@contextmanager +def concurrent_open(path, mode='r'): + """Ensure that complete file is read/written + + Works only for r, rb, w, w+, wb, wb+ modes. Default is r. + + In read mode behaves the same as build-in open. In write mode all data are + written to the temporary file first and then moved to target path. + This approach ensures that reading will be performed on complete file and + writting will result in complete or none file written at all. + """ + if mode in ('w', 'w+', 'wb', 'wb+'): + with tempfile.NamedTemporaryFile( + mode=mode, prefix=path, delete=False) as temp_file: + yield temp_file + temp_file.flush() + os.fsync(temp_file.fileno()) + try: + os.rename(temp_file.name, path) + except EnvironmentError: + os.remove(temp_file.name) + raise + elif mode in ('r', 'rb'): + with open(path, mode) as f: + yield f + else: + raise ValueError(u"mode string must be one of 'r', 'rb', 'w', 'w+', " + u"'wb', 'wb+'") + + def shell_quote(string): if isinstance(string, str): return "'" + string.replace("'", "'\\''") + "'"
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code