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] 467ed75ee1
[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
This commit is contained in:
Balazs Gibizer 2022-09-03 15:35:35 +02:00
parent 796203c948
commit ee3f73a133
2 changed files with 26 additions and 19 deletions

View File

@ -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
"""
.. 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
InterProcessLock = fasteners.InterProcessLock
class FairLocks(object):

View File

@ -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._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
# )