Skip to content

Commit e789b30

Browse files
committed
Merge bitcoin#27116: doc: clarify that LOCK() internally checks whether the mutex is held
91d0888 sync: unpublish LocksHeld() which is used only in sync.cpp (Vasil Dimov) 3df37e0 doc: clarify that LOCK() does AssertLockNotHeld() internally (Vasil Dimov) Pull request description: Constructs like ```cpp AssertLockNotHeld(m); LOCK(m); ``` are equivalent to (almost, modulo some logging differences, see below) ```cpp LOCK(m); ``` for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case. Also remove `LocksHeld()` from the public interface in `sync.h` since it is only used in `sync.cpp`. ACKs for top commit: achow101: ACK 91d0888 hebasto: ACK 91d0888, I have reviewed the code and it looks OK. Tree-SHA512: c4b7ef2c0bfeb28d1c4f55f497810f629873137e02f5a92137c02cb1ff603ac76473dcd2171e594491494a5cb87b8c0c803e06b86f190d4acb231791e28e802d
2 parents b72cb78 + 91d0888 commit e789b30

File tree

3 files changed

+4
-3
lines changed

3 files changed

+4
-3
lines changed

doc/developer-notes.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,9 @@ Threads and synchronization
950950
internal to a class (private or protected) rather than public.
951951
952952
- Combine annotations in function declarations with run-time asserts in
953-
function definitions:
953+
function definitions (`AssertLockNotHeld()` can be omitted if `LOCK()` is
954+
called unconditionally after it because `LOCK()` does the same check as
955+
`AssertLockNotHeld()` internally, for non-recursive mutexes):
954956
955957
```C++
956958
// txmempool.h

src/sync.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ void LeaveCritical()
246246
pop_lock();
247247
}
248248

249-
std::string LocksHeld()
249+
static std::string LocksHeld()
250250
{
251251
LockData& lockdata = GetLockData();
252252
std::lock_guard<std::mutex> lock(lockdata.dd_mutex);

src/sync.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ template <typename MutexType>
5757
void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false);
5858
void LeaveCritical();
5959
void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line);
60-
std::string LocksHeld();
6160
template <typename MutexType>
6261
void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) EXCLUSIVE_LOCKS_REQUIRED(cs);
6362
template <typename MutexType>

0 commit comments

Comments
 (0)