diff --git a/oslo_log/pipe_mutex.py b/oslo_log/pipe_mutex.py index 8fbab699..825735ed 100644 --- a/oslo_log/pipe_mutex.py +++ b/oslo_log/pipe_mutex.py @@ -43,6 +43,21 @@ class PipeMutex(object): self.owner = None self.recursion_depth = 0 + # Usually, it's an error to have multiple greenthreads all waiting + # to read the same file descriptor. It's often a sign of inadequate + # concurrency control; for example, if you have two greenthreads + # trying to use the same memcache connection, they'll end up writing + # interleaved garbage to the socket or stealing part of each others' + # responses. + # + # In this case, we have multiple greenthreads waiting on the same + # file descriptor by design. This lets greenthreads in real thread A + # wait with greenthreads in real thread B for the same mutex. + # Therefore, we must turn off eventlet's multiple-reader detection. + # + # It would be better to turn off multiple-reader detection for only + # our calls to trampoline(), but eventlet does not support that. + eventlet.debug.hub_prevent_multiple_readers(False) def acquire(self, blocking=True): """Acquire the mutex. diff --git a/oslo_log/tests/unit/test_pipe_mutex.py b/oslo_log/tests/unit/test_pipe_mutex.py index 4ff5d4de..c7b75915 100644 --- a/oslo_log/tests/unit/test_pipe_mutex.py +++ b/oslo_log/tests/unit/test_pipe_mutex.py @@ -20,7 +20,6 @@ from unittest import mock import eventlet from eventlet import debug as eventlet_debug from eventlet import greenpool -from eventlet import tpool from oslo_log import pipe_mutex @@ -36,7 +35,7 @@ def quiet_eventlet_exceptions(): class TestPipeMutex(unittest.TestCase): - """From Swift's test/unit/common/test_utils.py""" + """From Swift's test/unit/common/test_utils.py""" def setUp(self): self.mutex = pipe_mutex.PipeMutex() @@ -142,11 +141,18 @@ class TestPipeMutex(unittest.TestCase): greenthread1 = eventlet.spawn(do_stuff) greenthread2 = eventlet.spawn(do_stuff) - _ = tpool.execute(do_stuff) - _ = tpool.execute(do_stuff) + real_thread1 = eventlet.patcher.original('threading').Thread( + target=do_stuff) + real_thread1.start() + + real_thread2 = eventlet.patcher.original('threading').Thread( + target=do_stuff) + real_thread2.start() greenthread1.wait() greenthread2.wait() + real_thread1.join() + real_thread2.join() self.assertEqual(''.join(sequence), "<>" * 40) diff --git a/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml b/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml deleted file mode 100644 index 3b8d1d3b..00000000 --- a/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml +++ /dev/null @@ -1,14 +0,0 @@ ---- -fixes: - - | - Remove the usage of the Eventlet debug feature from oslo.log - Then hub_prevent_multiple_readers is a debug convenience. This - feature is a debug convenience. The problem with disabling this - procedure is that `it exposes you to risks `_. - Deactivation applies to the entire stack. If a project uses oslo.log, - for example nova, then it exposes all threads to concurrent access on - the process file descriptors. When several greenlets are reading from - the same socket, it's difficult to predict which greenlet will receive - which data. That services using oslo.log should prefer using tpool - rather than un-monkey patched version of stdlib threading module which - is not compatible with eventlet.