diff options
author | Matt Davis <6775756+nitzmahone@users.noreply.github.com> | 2022-12-05 20:46:15 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-05 20:46:15 -0800 |
commit | 1424484be0e1b9a1d1e7e1849ae1a5e2a19d612c (patch) | |
tree | d14e1296fc4ecf6202d78b79e6262c0c1728c000 /lib/ansible/utils/display.py | |
parent | 80d2f8da02052f64396da6b8caaf820eedbf18e2 (diff) | |
download | ansible-1424484be0e1b9a1d1e7e1849ae1a5e2a19d612c.tar.gz |
Prevent stdio deadlock in forked children (#79522)
* background threads writing to stdout/stderr can cause children to deadlock if a thread in the parent holds the internal lock on the BufferedWriter wrapper
* prevent writes to std handles during fork by monkeypatching stdout/stderr during display startup to require a mutex lock with fork(); this ensures no background threads can hold the lock during a fork operation
* add integration test that fails reliably on Linux without this fix
Diffstat (limited to 'lib/ansible/utils/display.py')
-rw-r--r-- | lib/ansible/utils/display.py | 29 |
1 files changed, 29 insertions, 0 deletions
diff --git a/lib/ansible/utils/display.py b/lib/ansible/utils/display.py index c3a5de98e2..e521f2a401 100644 --- a/lib/ansible/utils/display.py +++ b/lib/ansible/utils/display.py @@ -41,6 +41,7 @@ from ansible.utils.color import stringc from ansible.utils.multiprocessing import context as multiprocessing_context from ansible.utils.singleton import Singleton from ansible.utils.unsafe_proxy import wrap_var +from functools import wraps _LIBC = ctypes.cdll.LoadLibrary(ctypes.util.find_library('c')) @@ -163,12 +164,33 @@ b_COW_PATHS = ( ) +def _synchronize_textiowrapper(tio, lock): + # Ensure that a background thread can't hold the internal buffer lock on a file object + # during a fork, which causes forked children to hang. We're using display's existing lock for + # convenience (and entering the lock before a fork). + def _wrap_with_lock(f, lock): + @wraps(f) + def locking_wrapper(*args, **kwargs): + with lock: + return f(*args, **kwargs) + + return locking_wrapper + + buffer = tio.buffer + + # monkeypatching the underlying file-like object isn't great, but likely safer than subclassing + buffer.write = _wrap_with_lock(buffer.write, lock) + buffer.flush = _wrap_with_lock(buffer.flush, lock) + + class Display(metaclass=Singleton): def __init__(self, verbosity=0): self._final_q = None + # NB: this lock is used to both prevent intermingled output between threads and to block writes during forks. + # Do not change the type of this lock or upgrade to a shared lock (eg multiprocessing.RLock). self._lock = threading.RLock() self.columns = None @@ -199,6 +221,13 @@ class Display(metaclass=Singleton): self._set_column_width() + try: + # NB: we're relying on the display singleton behavior to ensure this only runs once + _synchronize_textiowrapper(sys.stdout, self._lock) + _synchronize_textiowrapper(sys.stderr, self._lock) + except Exception as ex: + self.warning(f"failed to patch stdout/stderr for fork-safety: {ex}") + def set_queue(self, queue): """Set the _final_q on Display, so that we know to proxy display over the queue instead of directly writing to stdout/stderr from forks |