URL: https://github.com/freeipa/freeipa/pull/467
Author: dkupka
 Title: #467: ipaclient: schema cache: Write all schema files in 
concurrent-safe way
Action: synchronized

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 31bd8d2b21160dfb7ad535c1c5521f0174948547 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   | 15 +++------------
 ipapython/ipautil.py                 | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 13 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..36ba741 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -5,8 +5,6 @@
 import collections
 import contextlib
 import errno
-import fcntl
-import io
 import json
 import os
 import sys
@@ -25,6 +23,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 +406,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..53bcf80 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -282,6 +282,41 @@ def write_tmp_file(txt):
 
     return fd
 
+
+@contextmanager
+def concurrent_open(filename, 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+'):
+        directory, basename = os.path.split(os.path.abspath(filename))
+        with tempfile.NamedTemporaryFile(
+                mode=mode, dir=directory, prefix=basename, delete=False
+        ) as temp_file:
+            try:
+                yield temp_file
+            finally:
+                temp_file.flush()
+                os.fsync(temp_file.fileno())
+                try:
+                    os.rename(temp_file.name, filename)
+                except EnvironmentError:
+                    os.unlink(temp_file.name)
+                    raise
+    elif mode in ('r', 'rb'):
+        with open(filename, 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

Reply via email to