================
@@ -51,6 +57,66 @@ bool ProcessRunLock::SetStopped() {
return was_running;
}
-} // namespace lldb_private
+#endif // !_WIN32
+
+/// Per-thread map of which ProcessRunLocks this thread already holds, with
+/// a recursion count. ProcessRunLocker uses this to detect re-entrant
+/// acquisitions and bump the count instead of recursively acquiring the
+/// underlying rwlock — recursive shared acquisition is either unsafe or
+/// implementation-defined, and on a write-preferring rwlock would deadlock
+/// against a queued writer.
+static llvm::DenseMap<ProcessRunLock *, uint32_t> &GetHeldLocks() {
+ static thread_local llvm::DenseMap<ProcessRunLock *, uint32_t> s_held;
----------------
medismailben wrote:
Tried this and it doesn't quite hold up — `single-std::atomic<thread::id>` can
only record one holder, but pthread_rwlock_t in read mode can have many
concurrent ones.
Concretely: `m_public_run_lock` regularly has multiple readers. The lldb
command interpreter, the DAP request handler, and any test/SB API client can
all enter SB API simultaneously, all succeed at `pthread_rwlock_rdlock`
(rwlocks allow concurrent readers), and now we have N legitimate holders for
one slot. Whichever thread wins the `compare_and_swap` becomes "the holder";
the others are real readers that the bookkeeping has no record of.
When one of those unrecorded readers later re-enters (e.g., it dispatches a
frame-provider Python callback that calls back into SB API), its TryLock checks
`m_holder == self`, sees the other thread's id, falls through to the real
`rdlock`, and we're back to the original deadlock — a queued writer (PST
SetStopped) blocks the inner reader exactly as before.
To track multiple concurrent readers we need a container, not a scalar. A
`DenseMap<thread::id, uint32_t>` per `ProcessRunLock` is what landed: same
colocation property as your suggestion (state lives in the lock, not in TLS),
but actually large enough to hold all current holders. Cost is one mutex + a
hash lookup per `TryLock`/`Unlock` — comparable to the global-TLS version, just
relocated.
I do agree the colocation is the right framing — that's the change from the
prior version. It just turned out the data structure had to grow with it.
https://github.com/llvm/llvm-project/pull/201685
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
