[Lldb-commits] [lldb] [lldb] Release output lock across blocking el_wgetc in DisplayCompletions (PR #196686)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

DisplayCompletions held m_output_stream_sp->Lock() across the blocking el_wgetc() call used by the "More (Y/n/a)" pager. Because the lock is a recursive_mutex, this worked when Editline::Interrupt() ran on the same thread (the synchronous SIGINT handler), but deadlocks when Interrupt() runs on another thread: it blocks on the lock and can never call InterruptRead() to wake the editor thread.

Mirror the pattern already used by Editline::GetCharacter: drop the lock across the blocking read and reacquire it afterward. The status check and the "^C\n" / "\n" prints stay under the lock.

---
Full diff: https://github.com/llvm/llvm-project/pull/196686.diff


1 Files Affected:

- (modified) lldb/source/Host/common/Editline.cpp (+16-6) 


``````````diff
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 39b0a649a7f60..833516b0b2c2d 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1054,9 +1054,10 @@ void Editline::DisplayCompletions(
     Editline &editline, llvm::ArrayRef<CompletionResult::Completion> results) {
   assert(!results.empty());
 
-  LockedStreamFile locked_stream = editline.m_output_stream_sp->Lock();
+  std::optional<LockedStreamFile> locked_stream =
+      editline.m_output_stream_sp->Lock();
 
-  fprintf(locked_stream.GetFile().GetStream(),
+  fprintf(locked_stream->GetFile().GetStream(),
           "\n" ANSI_CLEAR_BELOW "Available completions:\n");
 
   /// Account for the current line, the line showing "Available completions"
@@ -1075,14 +1076,20 @@ void Editline::DisplayCompletions(
   size_t cur_pos = 0;
   while (cur_pos < results.size()) {
     cur_pos += PrintCompletion(
-        locked_stream.GetFile().GetStream(), results.slice(cur_pos), max_len,
+        locked_stream->GetFile().GetStream(), results.slice(cur_pos), max_len,
         editline.GetTerminalWidth(),
         all ? std::nullopt : std::optional<size_t>(page_size));
 
     if (cur_pos >= results.size())
       break;
 
-    fprintf(locked_stream.GetFile().GetStream(), "More (Y/n/a): ");
+    fprintf(locked_stream->GetFile().GetStream(), "More (Y/n/a): ");
+
+    // Release the output lock across the blocking el_wgetc() so that
+    // Interrupt(), which may run on another thread, can acquire it to wake
+    // up the read.
+    locked_stream.reset();
+
     // The type for the output and the type for the parameter are different,
     // to allow interoperability with older versions of libedit. The container
     // for the reply must be as wide as what our implementation is using,
@@ -1091,14 +1098,17 @@ void Editline::DisplayCompletions(
     EditLineGetCharType reply = L'n';
     int got_char = el_wgetc(editline.m_editline,
                             reinterpret_cast<EditLineCharType *>(&reply));
+
+    locked_stream.emplace(editline.m_output_stream_sp->Lock());
+
     // Check for a ^C or other interruption.
     if (editline.m_editor_status == EditorStatus::Interrupted) {
       editline.m_editor_status = EditorStatus::Editing;
-      fprintf(locked_stream.GetFile().GetStream(), "^C\n");
+      fprintf(locked_stream->GetFile().GetStream(), "^C\n");
       break;
     }
 
-    fprintf(locked_stream.GetFile().GetStream(), "\n");
+    fprintf(locked_stream->GetFile().GetStream(), "\n");
     if (got_char == -1 || reply == 'n')
       break;
     if (reply == 'a')

``````````

</details>


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