[Lldb-commits] [lldb] Convert the StackFrameList mutex to a shared mutex. (PR #117252)

[via lldb-commits]

================
@@ -119,6 +120,7 @@ bool StackFrameList::DecrementCurrentInlinedDepth() {
     uint32_t current_inlined_depth = GetCurrentInlinedDepth();
     if (current_inlined_depth != UINT32_MAX) {
       if (current_inlined_depth > 0) {
+        std::lock_guard<std::mutex> guard(m_inlined_depth_mutex);
----------------
jimingham wrote:

That switch was necessitated by the change of `m_list_mutex` from a recursive to a non-recursive mutex.  This code often gets executed under some frame that has the list mutex.  So you can't just leave it as is.

The control of this feature isn't really well modeled by mutexes.  I think they were just there because recursive mutexes make it easy to reflexively put them around anything...

But the inlined depth of the current stack frame has to be set before we present the StackFrameList - it doesn't really make sense to do it otherwise.  BTW, that doesn't violate the "lazy stack frame building" aim because we only compute this for the inlined frames below the first concrete frame, which we always fetch.

So by the time any clients see the StackFrameList, the inlined depth should have been set.  It's not as simple as "has to be done in the constructor" because we also need to reset it at the point where we decide that we can implement a "step in" by just changing the inlined stack counter.  We do that without rebuilding the stack frame.

Currently this isn't a problem because we wrote the code so that it sets the inlined depth in the right place.  So perhaps we should just drop this mutex altogether and rely on this getting set and accessed in the right order, as currently happens?  But I'd rather do that as a separate patch.

https://github.com/llvm/llvm-project/pull/117252