Use locking for concurrent file access

changelogs/fragments/30413-lineinfile-concurrence_issue.yaml

@@ -0,0 +1,3 @@
+bugfixes:
+- change file locking implementation from a class to context manager to allow easy and safe concurrent file access by modules
+- lineinfile - lock on concurrent file access (https://github.com/ansible/ansible/issues/30413)

lib/ansible/module_utils/common/file.py

@@ -1,24 +1,21 @@
-# Copyright (c) 2018, Ansible Project
+# -*- coding: utf-8 -*-
+
+# Copyright: (c) 2018, Ansible Project
 # Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
 
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-import errno
+import fcntl
 import os
-import stat
 import re
-import pwd
-import grp
-import time
-import shutil
-import traceback
-import fcntl
+import stat
 import sys
+import time
 
 from contextlib import contextmanager
-from ansible.module_utils._text import to_bytes, to_native, to_text
-from ansible.module_utils.six import b, binary_type
+from ansible.module_utils._text import to_bytes
+from ansible.module_utils.six import PY3
 
 try:
     import selinux
@@ -62,6 +59,13 @@
 _DEFAULT_PERM = 0o0666       # default file permission bits
 
 
+# Ensure we use flock on e.g. FreeBSD, MacOSX and Solaris
+if sys.platform.startswith('linux'):
+    filelock = fcntl.lockf
+else:
+    filelock = fcntl.flock
+
+
 def is_executable(path):
     # This function's signature needs to be repeated
     # as the first line of its docstring.
@@ -114,89 +118,88 @@ class LockTimeout(Exception):
     pass
 
 
-class FileLock:
+# NOTE: Using the open_locked() context manager it is absolutely mandatory
+#       to not open or close the same file within the existing context.
+#       It is essential to reuse the returned file descriptor only.
+@contextmanager
+def open_locked(path, check_mode=False, lock_timeout=15):
+    '''
+    Context managed for opening files with lock acquisition
+
+    :kw path: Path (file) to lock
+    :kw lock_timeout:
+        Wait n seconds for lock acquisition, fail if timeout is reached.
+        0 = Do not wait, fail if lock cannot be acquired immediately,
+        Less than 0 or None = wait indefinitely until lock is released
+        Default is wait 15s.
+    :returns: file descriptor
+    '''
+    if check_mode:
+        b_path = to_bytes(path, errors='surrogate_or_strict')
+        fd = open(b_path, 'ab+')
+        fd.seek(0)  # Due to a difference in behavior between PY2 and PY3 we need to seek(0) on PY3
+    else:
+        fd = lock(path, check_mode, lock_timeout)
+    yield fd
+    fd.close()
+
+
+def lock(path, check_mode=False, lock_timeout=15):
+    '''
+    Set lock on given path via fcntl.flock(), note that using
+    locks does not guarantee exclusiveness unless all accessing
+    processes honor locks.
+
+    :kw path: Path (file) to lock
+    :kw lock_timeout:
+        Wait n seconds for lock acquisition, fail if timeout is reached.
+        0 = Do not wait, fail if lock cannot be acquired immediately,
+        Less than 0 or None = wait indefinitely until lock is released
+        Default is wait 15s.
+    :returns: file descriptor
     '''
-    Currently FileLock is implemented via fcntl.flock on a lock file, however this
-    behaviour may change in the future. Avoid mixing lock types fcntl.flock,
-    fcntl.lockf and module_utils.common.file.FileLock as it will certainly cause
-    unwanted and/or unexpected behaviour
+    b_path = to_bytes(path, errors='surrogate_or_strict')
+    wait = 0.1
+
+    lock_exception = IOError
+    if PY3:
+        lock_exception = OSError
+
+    if not os.path.exists(b_path):
+        raise IOError('{0} does not exist'.format(path))
+
+    if lock_timeout is None or lock_timeout < 0:
+        fd = open(b_path, 'ab+')
+        fd.seek(0)  # Due to a difference in behavior between PY2 and PY3 we need to seek(0) on PY3
+        filelock(fd, fcntl.LOCK_EX)
+        return fd
+
+    if lock_timeout >= 0:
+        total_wait = 0
+        while total_wait <= lock_timeout:
+            fd = open(b_path, 'ab+')
+            fd.seek(0)  # Due to a difference in behavior between PY2 and PY3 we need to seek(0) on PY3
+            try:
+                filelock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
+                return fd
+            except lock_exception:
+                fd.close()
+                time.sleep(wait)
+                total_wait += wait
+                continue
+
+        fd.close()
+        raise LockTimeout('Waited {0} seconds for lock on {1}'.format(total_wait, path))
+
+
+def unlock(fd):
+    '''
+    Make sure lock file is available for everyone and Unlock the file descriptor
+    locked by set_lock
+
+    :kw fd: File descriptor of file to unlock
     '''
-    def __init__(self):
-        self.lockfd = None
-
-    @contextmanager
-    def lock_file(self, path, tmpdir, lock_timeout=None):
-        '''
-        Context for lock acquisition
-        '''
-        try:
-            self.set_lock(path, tmpdir, lock_timeout)
-            yield
-        finally:
-            self.unlock()
-
-    def set_lock(self, path, tmpdir, lock_timeout=None):
-        '''
-        Create a lock file based on path with flock to prevent other processes
-        using given path.
-        Please note that currently file locking only works when it's executed by
-        the same user, I.E single user scenarios
-
-        :kw path: Path (file) to lock
-        :kw tmpdir: Path where to place the temporary .lock file
-        :kw lock_timeout:
-            Wait n seconds for lock acquisition, fail if timeout is reached.
-            0 = Do not wait, fail if lock cannot be acquired immediately,
-            Default is None, wait indefinitely until lock is released.
-        :returns: True
-        '''
-        lock_path = os.path.join(tmpdir, 'ansible-{0}.lock'.format(os.path.basename(path)))
-        l_wait = 0.1
-        r_exception = IOError
-        if sys.version_info[0] == 3:
-            r_exception = BlockingIOError
-
-        self.lockfd = open(lock_path, 'w')
-
-        if lock_timeout <= 0:
-            fcntl.flock(self.lockfd, fcntl.LOCK_EX | fcntl.LOCK_NB)
-            os.chmod(lock_path, stat.S_IWRITE | stat.S_IREAD)
-            return True
-
-        if lock_timeout:
-            e_secs = 0
-            while e_secs < lock_timeout:
-                try:
-                    fcntl.flock(self.lockfd, fcntl.LOCK_EX | fcntl.LOCK_NB)
-                    os.chmod(lock_path, stat.S_IWRITE | stat.S_IREAD)
-                    return True
-                except r_exception:
-                    time.sleep(l_wait)
-                    e_secs += l_wait
-                    continue
-
-            self.lockfd.close()
-            raise LockTimeout('{0} sec'.format(lock_timeout))
-
-        fcntl.flock(self.lockfd, fcntl.LOCK_EX)
-        os.chmod(lock_path, stat.S_IWRITE | stat.S_IREAD)
-
-        return True
-
-    def unlock(self):
-        '''
-        Make sure lock file is available for everyone and Unlock the file descriptor
-        locked by set_lock
-
-        :returns: True
-        '''
-        if not self.lockfd:
-            return True
-
-        try:
-            fcntl.flock(self.lockfd, fcntl.LOCK_UN)
-            self.lockfd.close()
-        except ValueError:  # file wasn't opened, let context manager fail gracefully
-            pass
-
-        return True
+    try:
+        filelock(fd, fcntl.LOCK_UN)
+    except ValueError:  # File was not opened, let context manager fail gracefully
+        pass