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

[Pavel Labath via lldb-commits]

================
@@ -578,72 +609,53 @@ void StackFrameList::Dump(Stream *s) {
 
 StackFrameSP StackFrameList::GetFrameAtIndex(uint32_t idx) {
   StackFrameSP frame_sp;
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
   uint32_t original_idx = idx;
 
-  uint32_t inlined_depth = GetCurrentInlinedDepth();
-  if (inlined_depth != UINT32_MAX)
-    idx += inlined_depth;
+  // We're going to consult the m_frames.size, but if there are already
+  // enough frames for our request we don't want to block other readers, so
+  // first acquire the shared lock:
+  { // Scope for shared lock:
+    std::shared_lock<std::shared_mutex> guard(m_list_mutex);
 
-  if (idx < m_frames.size())
-    frame_sp = m_frames[idx];
+    uint32_t inlined_depth = GetCurrentInlinedDepth();
+    if (inlined_depth != UINT32_MAX)
+      idx += inlined_depth;
 
-  if (frame_sp)
-    return frame_sp;
+    if (idx < m_frames.size())
+      frame_sp = m_frames[idx];
+
+    if (frame_sp)
+      return frame_sp;
+  } // End of reader lock scope
 
   // GetFramesUpTo will fill m_frames with as many frames as you asked for, if
   // there are that many.  If there weren't then you asked for too many frames.
   // GetFramesUpTo returns true if interrupted:
-  if (GetFramesUpTo(idx)) {
+  if (GetFramesUpTo(idx, AllowInterruption)) {
     Log *log = GetLog(LLDBLog::Thread);
     LLDB_LOG(log, "GetFrameAtIndex was interrupted");
     return {};
   }
 
-  if (idx < m_frames.size()) {
-    if (m_show_inlined_frames) {
-      // When inline frames are enabled we actually create all the frames in
-      // GetFramesUpTo.
-      frame_sp = m_frames[idx];
-    } else {
-      addr_t pc, cfa;
-      bool behaves_like_zeroth_frame = (idx == 0);
-      if (m_thread.GetUnwinder().GetFrameInfoAtIndex(
-              idx, cfa, pc, behaves_like_zeroth_frame)) {
-        const bool cfa_is_valid = true;
-        frame_sp = std::make_shared<StackFrame>(
-            m_thread.shared_from_this(), idx, idx, cfa, cfa_is_valid, pc,
-            StackFrame::Kind::Regular, behaves_like_zeroth_frame, nullptr);
-
-        Function *function =
-            frame_sp->GetSymbolContext(eSymbolContextFunction).function;
-        if (function) {
-          // When we aren't showing inline functions we always use the top
-          // most function block as the scope.
-          frame_sp->SetSymbolContextScope(&function->GetBlock(false));
-        } else {
-          // Set the symbol scope from the symbol regardless if it is nullptr
-          // or valid.
-          frame_sp->SetSymbolContextScope(
-              frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol);
-        }
-        SetFrameAtIndex(idx, frame_sp);
+  {  // Now we're accessing m_frames as a reader, so acquire the reader lock.
----------------
labath wrote:

This scope shouldn't be necessary. If I'm reading this correctly, we're returning directly afterwards.

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