[Lldb-commits] [lldb] 8e776bb - Re-land "[lldb] Synchronize output through the IOHandler"
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 15 12:53:55 PDT 2022
Author: Jonas Devlieghere
Date: 2022-03-15T12:53:46-07:00
New Revision: 8e776bb660dda6c51ce7ca6cea641db1f47aa9cf
URL: https://github.com/llvm/llvm-project/commit/8e776bb660dda6c51ce7ca6cea641db1f47aa9cf
DIFF: https://github.com/llvm/llvm-project/commit/8e776bb660dda6c51ce7ca6cea641db1f47aa9cf.diff
LOG: Re-land "[lldb] Synchronize output through the IOHandler"
Add synchronization to the IOHandler to prevent multiple threads from
writing concurrently to the output or error stream.
A scenario where this could happen is when a thread (the default event
thread for example) is using the debugger's asynchronous stream. We
would delegate this operation to the IOHandler which might be running on
another thread. Until this patch there was nothing to synchronize the
two at the IOHandler level.
Differential revision: https://reviews.llvm.org/D121500
Added:
Modified:
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Core/IOHandler.cpp
lldb/source/Host/common/Editline.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/unittests/Editline/EditlineTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/IOHandler.h b/lldb/include/lldb/Core/IOHandler.h
index d58ad68a099c6..2ba421f7ba506 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -34,7 +34,7 @@ class Debugger;
namespace repro {
class DataRecorder;
}
-}
+} // namespace lldb_private
namespace curses {
class Application;
@@ -165,11 +165,14 @@ class IOHandler {
virtual void PrintAsync(const char *s, size_t len, bool is_stdout);
+ std::recursive_mutex &GetOutputMutex() { return m_output_mutex; }
+
protected:
Debugger &m_debugger;
lldb::FileSP m_input_sp;
lldb::StreamFileSP m_output_sp;
lldb::StreamFileSP m_error_sp;
+ std::recursive_mutex m_output_mutex;
repro::DataRecorder *m_data_recorder;
Predicate<bool> m_popped;
Flags m_flags;
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index 14b6778d20764..ffe2e3500aaa2 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -154,7 +154,8 @@ using namespace line_editor;
class Editline {
public:
Editline(const char *editor_name, FILE *input_file, FILE *output_file,
- FILE *error_file, bool color_prompts);
+ FILE *error_file, std::recursive_mutex &output_mutex,
+ bool color_prompts);
~Editline();
@@ -402,7 +403,7 @@ class Editline {
std::string m_suggestion_ansi_suffix;
std::size_t m_previous_autosuggestion_size = 0;
- std::mutex m_output_mutex;
+ std::recursive_mutex &m_output_mutex;
};
}
diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 938d36ba0f3fc..787dfdbcb21f5 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -655,7 +655,8 @@ class CommandInterpreter : public Broadcaster,
const CommandObject::CommandMap &command_map);
// An interruptible wrapper around the stream output
- void PrintCommandOutput(Stream &stream, llvm::StringRef str);
+ void PrintCommandOutput(IOHandler &io_handler, llvm::StringRef str,
+ bool is_stdout);
bool EchoCommandNonInteractive(llvm::StringRef line,
const Flags &io_handler_flags) const;
diff --git a/lldb/source/Core/IOHandler.cpp b/lldb/source/Core/IOHandler.cpp
index 8ed2cfb22873a..44b5a7e9c8a40 100644
--- a/lldb/source/Core/IOHandler.cpp
+++ b/lldb/source/Core/IOHandler.cpp
@@ -123,6 +123,7 @@ void IOHandler::SetPopped(bool b) { m_popped.SetValue(b, eBroadcastOnChange); }
void IOHandler::WaitForPop() { m_popped.WaitForValueEqualTo(true); }
void IOHandler::PrintAsync(const char *s, size_t len, bool is_stdout) {
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
stream->Write(s, len);
stream->Flush();
@@ -266,9 +267,9 @@ IOHandlerEditline::IOHandlerEditline(
m_input_sp && m_input_sp->GetIsRealTerminal();
if (use_editline) {
- m_editline_up = std::make_unique<Editline>(editline_name, GetInputFILE(),
- GetOutputFILE(), GetErrorFILE(),
- m_color_prompts);
+ m_editline_up = std::make_unique<Editline>(
+ editline_name, GetInputFILE(), GetOutputFILE(), GetErrorFILE(),
+ GetOutputMutex(), m_color_prompts);
m_editline_up->SetIsInputCompleteCallback(
[this](Editline *editline, StringList &lines) {
return this->IsInputCompleteCallback(editline, lines);
@@ -619,6 +620,7 @@ void IOHandlerEditline::GotEOF() {
void IOHandlerEditline::PrintAsync(const char *s, size_t len, bool is_stdout) {
#if LLDB_ENABLE_LIBEDIT
if (m_editline_up) {
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
lldb::StreamFileSP stream = is_stdout ? m_output_sp : m_error_sp;
m_editline_up->PrintAsync(stream.get(), s, len);
} else
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index 826db61f19a6b..ea0f06f637071 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1376,10 +1376,12 @@ Editline *Editline::InstanceFor(EditLine *editline) {
}
Editline::Editline(const char *editline_name, FILE *input_file,
- FILE *output_file, FILE *error_file, bool color_prompts)
+ FILE *output_file, FILE *error_file,
+ std::recursive_mutex &output_mutex, bool color_prompts)
: m_editor_status(EditorStatus::Complete), m_color_prompts(color_prompts),
m_input_file(input_file), m_output_file(output_file),
- m_error_file(error_file), m_input_connection(fileno(input_file), false) {
+ m_error_file(error_file), m_input_connection(fileno(input_file), false),
+ m_output_mutex(output_mutex) {
// Get a shared history instance
m_editor_name = (editline_name == nullptr) ? "lldb-tmp" : editline_name;
m_history_sp = EditlineHistory::GetHistory(m_editor_name);
@@ -1388,12 +1390,13 @@ Editline::Editline(const char *editline_name, FILE *input_file,
if (m_output_file) {
const int term_fd = fileno(m_output_file);
if (term_fd != -1) {
- static std::mutex *g_init_terminal_fds_mutex_ptr = nullptr;
+ static std::recursive_mutex *g_init_terminal_fds_mutex_ptr = nullptr;
static std::set<int> *g_init_terminal_fds_ptr = nullptr;
static llvm::once_flag g_once_flag;
llvm::call_once(g_once_flag, [&]() {
g_init_terminal_fds_mutex_ptr =
- new std::mutex(); // NOTE: Leak to avoid C++ destructor chain issues
+ new std::recursive_mutex(); // NOTE: Leak to avoid C++ destructor
+ // chain issues
g_init_terminal_fds_ptr = new std::set<int>(); // NOTE: Leak to avoid
// C++ destructor chain
// issues
@@ -1401,7 +1404,8 @@ Editline::Editline(const char *editline_name, FILE *input_file,
// We must make sure to initialize the terminal a given file descriptor
// only once. If we do this multiple times, we start leaking memory.
- std::lock_guard<std::mutex> guard(*g_init_terminal_fds_mutex_ptr);
+ std::lock_guard<std::recursive_mutex> guard(
+ *g_init_terminal_fds_mutex_ptr);
if (g_init_terminal_fds_ptr->find(term_fd) ==
g_init_terminal_fds_ptr->end()) {
g_init_terminal_fds_ptr->insert(term_fd);
@@ -1473,7 +1477,7 @@ uint32_t Editline::GetCurrentLine() { return m_current_line_index; }
bool Editline::Interrupt() {
bool result = true;
- std::lock_guard<std::mutex> guard(m_output_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
if (m_editor_status == EditorStatus::Editing) {
fprintf(m_output_file, "^C\n");
result = m_input_connection.InterruptRead();
@@ -1484,7 +1488,7 @@ bool Editline::Interrupt() {
bool Editline::Cancel() {
bool result = true;
- std::lock_guard<std::mutex> guard(m_output_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
if (m_editor_status == EditorStatus::Editing) {
MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockStart);
fprintf(m_output_file, ANSI_CLEAR_BELOW);
@@ -1499,7 +1503,7 @@ bool Editline::GetLine(std::string &line, bool &interrupted) {
m_input_lines = std::vector<EditLineStringType>();
m_input_lines.insert(m_input_lines.begin(), EditLineConstString(""));
- std::lock_guard<std::mutex> guard(m_output_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
lldbassert(m_editor_status != EditorStatus::Editing);
if (m_editor_status == EditorStatus::Interrupted) {
@@ -1544,7 +1548,7 @@ bool Editline::GetLines(int first_line_number, StringList &lines,
m_input_lines = std::vector<EditLineStringType>();
m_input_lines.insert(m_input_lines.begin(), EditLineConstString(""));
- std::lock_guard<std::mutex> guard(m_output_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
// Begin the line editing loop
DisplayInput();
SetCurrentLine(0);
@@ -1574,7 +1578,7 @@ bool Editline::GetLines(int first_line_number, StringList &lines,
}
void Editline::PrintAsync(Stream *stream, const char *s, size_t len) {
- std::lock_guard<std::mutex> guard(m_output_mutex);
+ std::lock_guard<std::recursive_mutex> guard(m_output_mutex);
if (m_editor_status == EditorStatus::Editing) {
MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockStart);
fprintf(m_output_file, ANSI_CLEAR_BELOW);
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index edf4f59a6b7bb..acd0a71c9da32 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2975,8 +2975,12 @@ bool CommandInterpreter::WasInterrupted() const {
return was_interrupted;
}
-void CommandInterpreter::PrintCommandOutput(Stream &stream,
- llvm::StringRef str) {
+void CommandInterpreter::PrintCommandOutput(IOHandler &io_handler,
+ llvm::StringRef str,
+ bool is_stdout) {
+
+ lldb::StreamFileSP stream = is_stdout ? io_handler.GetOutputStreamFileSP()
+ : io_handler.GetErrorStreamFileSP();
// Split the output into lines and poll for interrupt requests
const char *data = str.data();
size_t size = str.size();
@@ -2989,15 +2993,19 @@ void CommandInterpreter::PrintCommandOutput(Stream &stream,
break;
}
}
- chunk_size = stream.Write(data, chunk_size);
+ {
+ std::lock_guard<std::recursive_mutex> guard(io_handler.GetOutputMutex());
+ chunk_size = stream->Write(data, chunk_size);
+ }
lldbassert(size >= chunk_size);
data += chunk_size;
size -= chunk_size;
}
- if (size > 0) {
- stream.Printf("\n... Interrupted.\n");
- }
- stream.Flush();
+
+ std::lock_guard<std::recursive_mutex> guard(io_handler.GetOutputMutex());
+ if (size > 0)
+ stream->Printf("\n... Interrupted.\n");
+ stream->Flush();
}
bool CommandInterpreter::EchoCommandNonInteractive(
@@ -3033,9 +3041,11 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
// When using a non-interactive file handle (like when sourcing commands
// from a file) we need to echo the command out so we don't just see the
// command output and no command...
- if (EchoCommandNonInteractive(line, io_handler.GetFlags()))
+ if (EchoCommandNonInteractive(line, io_handler.GetFlags())) {
+ std::lock_guard<std::recursive_mutex> guard(io_handler.GetOutputMutex());
io_handler.GetOutputStreamFileSP()->Printf(
"%s%s\n", io_handler.GetPrompt(), line.c_str());
+ }
}
StartHandlingCommand();
@@ -3057,13 +3067,13 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
if (!result.GetImmediateOutputStream()) {
llvm::StringRef output = result.GetOutputData();
- PrintCommandOutput(*io_handler.GetOutputStreamFileSP(), output);
+ PrintCommandOutput(io_handler, output, true);
}
// Now emit the command error text from the command we just executed
if (!result.GetImmediateErrorStream()) {
llvm::StringRef error = result.GetErrorData();
- PrintCommandOutput(*io_handler.GetErrorStreamFileSP(), error);
+ PrintCommandOutput(io_handler, error, false);
}
}
diff --git a/lldb/unittests/Editline/EditlineTest.cpp b/lldb/unittests/Editline/EditlineTest.cpp
index 3a1aba4684ccd..92788374bc628 100644
--- a/lldb/unittests/Editline/EditlineTest.cpp
+++ b/lldb/unittests/Editline/EditlineTest.cpp
@@ -84,6 +84,7 @@ class EditlineAdapter {
bool IsInputComplete(lldb_private::Editline *editline,
lldb_private::StringList &lines);
+ std::recursive_mutex output_mutex;
std::unique_ptr<lldb_private::Editline> _editline_sp;
PseudoTerminal _pty;
@@ -117,7 +118,7 @@ EditlineAdapter::EditlineAdapter()
// Create an Editline instance.
_editline_sp.reset(new lldb_private::Editline(
"gtest editor", *_el_secondary_file, *_el_secondary_file,
- *_el_secondary_file, false));
+ *_el_secondary_file, output_mutex, false));
_editline_sp->SetPrompt("> ");
// Hookup our input complete callback.
More information about the lldb-commits
mailing list