diff --git a/oslo_log/pipe_mutex.py b/oslo_log/pipe_mutex.py index 825735ed..8fbab699 100644 --- a/oslo_log/pipe_mutex.py +++ b/oslo_log/pipe_mutex.py @@ -43,21 +43,6 @@ 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 c7b75915..4ff5d4de 100644 --- a/oslo_log/tests/unit/test_pipe_mutex.py +++ b/oslo_log/tests/unit/test_pipe_mutex.py @@ -20,6 +20,7 @@ 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 @@ -35,7 +36,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() @@ -141,18 +142,11 @@ class TestPipeMutex(unittest.TestCase): greenthread1 = eventlet.spawn(do_stuff) greenthread2 = eventlet.spawn(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() + _ = tpool.execute(do_stuff) + _ = tpool.execute(do_stuff) 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 new file mode 100644 index 00000000..958805b1 --- /dev/null +++ b/releasenotes/notes/remove-eventlet-hub_prevent_multiple_readers-usage-b23cea69a05cd6d4.yaml @@ -0,0 +1,14 @@ +--- +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 risk. 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.