summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBalazs Gibizer <gibi@redhat.com>2022-09-03 15:35:35 +0200
committerBalazs Gibizer <gibi@redhat.com>2022-09-05 13:29:04 +0200
commitee3f73a13379a79282325906787aae7da0f6ac27 (patch)
tree9f421d51d3407f6b3f8e762cda6116d66cfcc51e
parent796203c94846d44237d869e3b20a6cd8a3602e36 (diff)
downloadoslo-concurrency-ee3f73a13379a79282325906787aae7da0f6ac27.tar.gz
Fix fair internal lock used from eventlet.spawn_n
The fasteners lib in version 0.15.0 removed the threading.current_thread workaround for eventlet[1] because eventlet seemed to fixed the current_thread issues tracked in [2]. However the fix for [2] only fixed half of the problem. The threading.current_thread call works if it is called from thread created by eventlet.spawn. However if the thread is created with eventlet.spawn_n then threading.current_thread is still broken and returns the ID of the python native thread. The fasteners' ReaderWriterLock depends heavily on threading.current_thread to decide which thread holds a lock and to allow re-entry of that thread. This leads to the situation that multiple threads created from spawn_n could take the same ReaderWriterLock at the same time. The fair internal lock in oslo.concurrency uses ReaderWriterLock and as a result such lock is broken for threads created with spawn_n. Note that this issue was raised with eventlet in [3] when the nova team detected it via a direct usage of ReaderWriterLock in the nova test code. As [3] did not lead to a solution in eventlet nova implemented a nova local fix for the test code in [4]. However now we detected that oslo.concurrency is affected by this issue as well. This patch restores the workaround that was removed by [1]. Note that a fasteners issue [5] also opened to restore the workaround[1]. [1] https://github.com/harlowja/fasteners/commit/467ed75ee1e9465ebff8b5edf452770befb93913 [2] https://github.com/eventlet/eventlet/issues/172 [3] https://github.com/eventlet/eventlet/issues/731 [4] https://review.opendev.org/c/openstack/nova/+/813114 [5] https://github.com/harlowja/fasteners/issues/96 Closes-Bug: #1988311 Change-Id: Ia873bcc6b07121c9bd0b94c593567d537b4c1112
-rw-r--r--oslo_concurrency/lockutils.py29
-rw-r--r--oslo_concurrency/tests/unit/test_lockutils_eventlet.py16
2 files changed, 26 insertions, 19 deletions
diff --git a/oslo_concurrency/lockutils.py b/oslo_concurrency/lockutils.py
index 4b085c4..8ca6d1a 100644
--- a/oslo_concurrency/lockutils.py
+++ b/oslo_concurrency/lockutils.py
@@ -32,6 +32,14 @@ from oslo_utils import timeutils
from oslo_concurrency._i18n import _
+try:
+ # import eventlet optionally
+ import eventlet
+ from eventlet import patcher as eventlet_patcher
+except ImportError:
+ eventlet = None
+ eventlet_patcher = None
+
LOG = logging.getLogger(__name__)
@@ -79,12 +87,23 @@ def get_lock_path(conf):
return conf.oslo_concurrency.lock_path
-InterProcessLock = fasteners.InterProcessLock
-ReaderWriterLock = fasteners.ReaderWriterLock
-"""A reader/writer lock.
+class ReaderWriterLock(fasteners.ReaderWriterLock):
+ """A reader/writer lock.
+
+ .. versionadded:: 0.4
+ """
+ def __init__(self, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+ # Until https://github.com/eventlet/eventlet/issues/731 is resolved
+ # we need to use eventlet.getcurrent instead of
+ # threading.current_thread if we are running in a monkey patched
+ # environment
+ if eventlet is not None and eventlet_patcher is not None:
+ if eventlet_patcher.is_monkey_patched('thread'):
+ self._current_thread = eventlet.getcurrent
+
-.. versionadded:: 0.4
-"""
+InterProcessLock = fasteners.InterProcessLock
class FairLocks(object):
diff --git a/oslo_concurrency/tests/unit/test_lockutils_eventlet.py b/oslo_concurrency/tests/unit/test_lockutils_eventlet.py
index 24fd7f6..ad51f03 100644
--- a/oslo_concurrency/tests/unit/test_lockutils_eventlet.py
+++ b/oslo_concurrency/tests/unit/test_lockutils_eventlet.py
@@ -99,18 +99,6 @@ class TestInternalLock(test_base.BaseTestCase):
)
def test_fair_lock_with_spawn_n(self):
- # This is bug 1988311 where spawn_n does not work with fair locks
- ex = self.assertRaises(
- AssertionError,
- self._test_internal_lock_with_two_threads,
- fair=True,
- spawn=eventlet.spawn_n,
- )
- self.assertEqual(
- "'finished' is not None: "
- "Two threads was able to take the same lock",
- str(ex),
+ self._test_internal_lock_with_two_threads(
+ fair=True, spawn=eventlet.spawn_n
)
- # self._test_internal_lock_with_two_threads(
- # fair=True, spawn=eventlet.spawn_n
- # )