diff options
author | Balazs Gibizer <gibi@redhat.com> | 2022-09-03 15:35:35 +0200 |
---|---|---|
committer | Balazs Gibizer <gibi@redhat.com> | 2022-09-05 13:29:04 +0200 |
commit | ee3f73a13379a79282325906787aae7da0f6ac27 (patch) | |
tree | 9f421d51d3407f6b3f8e762cda6116d66cfcc51e | |
parent | 796203c94846d44237d869e3b20a6cd8a3602e36 (diff) | |
download | oslo-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.py | 29 | ||||
-rw-r--r-- | oslo_concurrency/tests/unit/test_lockutils_eventlet.py | 16 |
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 - # ) |