womax/nova
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "FairLockGuard: Support cross-thread sharing and nesting"

Browse Source
This commit is contained in:
Zuul
2026-02-18 17:54:57 +00:00
committed by Gerrit Code Review
parent 0d074c6775 72132f89ee
commit 7dd05b7af6
2 changed files with 165 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nova/tests/unit/test_utils.py
+69 -9
View File
@@ -21,6 +21,7 @@ import threading
import time
from unittest import mock
import fasteners
import fixtures
from keystoneauth1 import adapter as ks_adapter
from keystoneauth1.identity import base as ks_identity
@@ -1997,22 +1998,81 @@ class TestFairLockGuard(test.NoDBTestCase):
self.assertTrue(test_locks[0].is_writer())
self.assertTrue(test_locks[1].is_writer())
# attempting to nest the same context manager instance
# should raise a TypeError.
# Same instance nesting now works - locks are already held,
# so nested entry just increments depth counter.
with lock_guard:
self.assertTrue(lock_guard.is_locked())
self.assertTrue(test_locks[0].is_writer())
self.assertTrue(test_locks[1].is_writer())
with self.assertRaisesRegex(
TypeError,
"Cannot enter FairLockGuard while it is already active."):
with lock_guard:
pass
# after the TypeError, the outer context should still
# be active.
with lock_guard:
# Still locked in nested context
self.assertTrue(lock_guard.is_locked())
self.assertTrue(test_locks[0].is_writer())
self.assertTrue(test_locks[1].is_writer())
# Still locked after inner exit
self.assertTrue(lock_guard.is_locked())
self.assertTrue(test_locks[0].is_writer())
self.assertTrue(test_locks[1].is_writer())
# Released after outer exit
self.assertFalse(lock_guard.is_locked())
self.assertFalse(test_locks[0].is_writer())
self.assertFalse(test_locks[1].is_writer())
@mock.patch.object(utils, 'NOVA_FAIR_LOCKS')
def test_deep_nesting(self, mock_fair_locks):
"""Test that nesting works correctly at 3+ levels deep."""
test_lock = fasteners.ReaderWriterLock()
mock_fair_locks.get.return_value = test_lock
lock_guard = utils.FairLockGuard(['deep-test'])
with lock_guard:
self.assertTrue(lock_guard.is_locked())
with lock_guard:
self.assertTrue(lock_guard.is_locked())
with lock_guard:
# Third level of nesting
self.assertTrue(lock_guard.is_locked())
self.assertTrue(test_lock.is_writer())
# Still locked after level 3 exit
self.assertTrue(lock_guard.is_locked())
# Still locked after level 2 exit
self.assertTrue(lock_guard.is_locked())
# Released after outermost exit
self.assertFalse(lock_guard.is_locked())
self.assertFalse(test_lock.is_writer())
@mock.patch.object(utils, 'NOVA_FAIR_LOCKS')
def test_nested_exception_outer_still_holds_locks(self, mock_fair_locks):
"""Test that outer context retains locks when inner context raises."""
test_lock = fasteners.ReaderWriterLock()
mock_fair_locks.get.return_value = test_lock
lock_guard = utils.FairLockGuard(['exception-test'])
with lock_guard:
self.assertTrue(lock_guard.is_locked())
try:
with lock_guard:
self.assertTrue(lock_guard.is_locked())
raise ValueError("Test exception")
except ValueError:
pass
# Outer context still holds locks after inner raised
self.assertTrue(lock_guard.is_locked())
self.assertTrue(test_lock.is_writer())
# Released after outer exit
self.assertFalse(lock_guard.is_locked())
self.assertFalse(test_lock.is_writer())
@mock.patch.object(utils, 'NOVA_FAIR_LOCKS')
def test_empty_lock_list(self, mock_fair_locks):
"""Test FairLockGuard with empty lock list."""
lock_guard = utils.FairLockGuard([])
with lock_guard:
# No locks to acquire, but should still work
self.assertFalse(lock_guard.is_locked())
mock_fair_locks.get.assert_not_called()
class StaticallyDelayingCancellableTaskExecutorWrapperTest(test.NoDBTestCase):
nova/utils.py
+96 -33
View File
@@ -1172,7 +1172,7 @@ def latch_error_on_raise(retryable=(_SentinelException,)):
class FairLockGuard:
"""A lock guard context manager
This class support acquiring multiple locks safely by name
This class supports acquiring multiple locks safely by name
and releasing them via the context manager protocol
i.e. with FairLockGuard([list of lock names]):
@@ -1181,33 +1181,42 @@ class FairLockGuard:
the locks and passing them in. Lock creation and management
is handled entirely internally.
If you are using this between threads, Thread-A and Thread-B
should both create there own context manager instead of sharing
a single context manager between treads.
Thread Safety:
While sharing a FairLockGuard instance between threads is
supported, it is discouraged. Each thread should create its
own context manager instance with the same set of lock names
for clarity and to avoid confusion about lock ownership. If
shared, threads will correctly wait for each other due to the
underlying fair locks.
Nesting is supported by creating a new context manager instance
for each nested context with the same or different lock names
as the outer context. Attempting to nest the same context manager
instance should raise a TypeError.
Nesting:
Same-instance nesting is supported but discouraged. When the
same thread re-enters an active context manager, the nesting
depth is tracked and locks are only released on the outermost
exit. For clarity, prefer creating separate FairLockGuard
instances for nested contexts.
Example Valid Usage:
Example Recommended Usage:
```
with FairLockGuard(['lock1', 'lock2']) as lock_guard:
with FairLockGuard(['lock1', 'lock2']) as lock_guard2:
# Each context creates its own guard - clear ownership
with FairLockGuard(['lock1', 'lock2']):
with FairLockGuard(['lock1', 'lock2']):
pass
```
Example Invalid Usage:
Example Discouraged (but supported) Usage:
```
with FairLockGuard(['lock1', 'lock2']) as lock_guard:
with lock_guard:
pass
# Same instance nesting - works but less clear
lock_guard = FairLockGuard(['lock1', 'lock2'])
with lock_guard:
with lock_guard: # Increments nesting depth
pass # Decrements but doesn't release
pass # Still holds locks
# Locks released here
```
This will raise a TypeError because the same context manager instance
is being nested.
In general you should avoid naming the context manager instance
and only construct it in the with statement to avoid incorrect usage.
and only construct it in the with statement to keep usage clear.
"""
def __init__(self, names):
@@ -1227,33 +1236,87 @@ class FairLockGuard:
# exclusive access for state modifications.
self.locks_lock = lockutils.ReaderWriterLock()
self._active = False
self._active_thread = None
self._nesting_depth = 0
@staticmethod
def _release_locks(locks):
for lock in locks:
if lock.is_writer():
try:
lock.release_write_lock()
except Exception:
pass
def __enter__(self):
current_thread = threading.current_thread()
# Check for same-thread nesting - allow it by incrementing depth
with self.locks_lock.write_lock():
if self._active:
raise TypeError(
"Cannot enter FairLockGuard while it is already active. "
"Create a new instance for nested usage or wait for the "
"current context to exit.")
if self._active and self._active_thread == current_thread:
# Same thread re-entering - just increment depth.
# Locks are already held, no need to re-acquire.
self._nesting_depth += 1
return
# First entry (or different thread that will wait on named locks).
# IMPORTANT: Acquire named locks OUTSIDE of locks_lock to prevent
# deadlock. If we acquired named locks while holding locks_lock:
# 1. Thread-A holds locks_lock, acquires named locks, releases
# locks_lock, does work
# 2. Thread-B takes locks_lock, blocks waiting for named locks
# (still holding locks_lock)
# 3. Thread-A tries to __exit__, needs locks_lock - DEADLOCK!
# By acquiring named locks outside locks_lock, Thread-B blocks on
# named locks without holding locks_lock, allowing Thread-A to exit.
#
# NOTE: We append to acquired_locks BEFORE calling
# acquire_write_lock().
# This ordering is intentional for exception safety: if acquire fails
# for a lock, we can still release all previously acquired locks
# (the is_writer() check ensures we only release locks that were
# actually acquired).
acquired_locks = []
try:
for name in self.names:
named_lock = NOVA_FAIR_LOCKS.get(name)
self.locks.append(named_lock)
# we ensure we add it to the list
# before we acquire the lock to make sure
# we release it if there is an exception
acquired_locks.append(named_lock)
named_lock.acquire_write_lock()
except Exception:
# Release any locks acquired so far on failure
self._release_locks(acquired_locks)
raise
# Update internal state after acquiring locks
with self.locks_lock.write_lock():
self.locks = acquired_locks
self._active = True
self._active_thread = current_thread
self._nesting_depth = 1
def __exit__(self, exc_type, exc_value, traceback):
current_thread = threading.current_thread()
with self.locks_lock.write_lock():
# Validate the exiting thread owns the lock. This handles the
# edge case where a shared guard's __exit__ is called from a
# thread that didn't successfully __enter__ (e.g., Thread-B
# calls exit while Thread-A holds nested locks). In normal
# context manager usage this shouldn't happen, but we handle
# it defensively by returning early without modifying state.
if self._active_thread != current_thread:
return
self._nesting_depth -= 1
if self._nesting_depth > 0:
# Still nested, don't release locks yet
return
# Release locks OUTSIDE of locks_lock to prevent holding it
# while other threads may be waiting
self._release_locks(self.locks)
with self.locks_lock.write_lock():
for lock in self.locks:
# This should always be true but since
# we add the lock to the list before we
# acquire it check anyway.
if lock.is_writer():
lock.release_write_lock()
self.locks = []
self._active = False
self._active_thread = None
def is_locked(self):
# NOTE(sean-k-mooney): LockGuards exist in several programming