Copilot commented on code in PR #13330:
URL: https://github.com/apache/trafficserver/pull/13330#discussion_r3471948253
##########
include/tsutil/Bravo.h:
##########
@@ -240,16 +174,16 @@ template <typename T = std::shared_mutex, size_t
SLOT_SIZE = 256, int SLOWDOWN_G
}
void
- unlock()
+ unlock() TS_RELEASE() TS_NO_THREAD_SAFETY_ANALYSIS
{
_mutex.underlying.unlock();
}
////
- // Shared locking
+ // Shared locking (bodies exempted for the same reason as the exclusive ones
above)
//
void
- lock_shared(Token &token)
+ lock_shared(Token &token) TS_ACQUIRE_SHARED() TS_NO_THREAD_SAFETY_ANALYSIS
{
debug_assert(SLOT_SIZE >= DenseThreadId::num_possible_values());
Review Comment:
`lock_shared(Token &token)` only assigns `token` on the fast path. If a
caller reuses a `Token` that previously held a non-zero fast-path value and
this call takes the slow path (e.g., after `_revoke()` clears `read_bias`),
`token` can retain that stale non-zero value. A subsequent
`unlock_shared(token)` would then incorrectly take the fast-path slot release
and fail to unlock the underlying shared_mutex, potentially deadlocking.
##########
include/tsutil/Bravo.h:
##########
@@ -277,7 +211,7 @@ template <typename T = std::shared_mutex, size_t SLOT_SIZE
= 256, int SLOWDOWN_G
}
bool
- try_lock_shared(Token &token)
+ try_lock_shared(Token &token) TS_TRY_ACQUIRE_SHARED(true)
TS_NO_THREAD_SAFETY_ANALYSIS
{
debug_assert(SLOT_SIZE >= DenseThreadId::num_possible_values());
Review Comment:
`try_lock_shared(Token &token)` only assigns `token` on the fast path. If
the slow path succeeds (or if it fails) and the caller passed a non-zero token
from a prior fast-path acquisition, `unlock_shared(token)` can later mis-handle
the unlock (treating it as a slot unlock instead of unlocking the underlying
mutex). Resetting `token` to 0 on entry makes the slow-path contract explicit
and robust.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
