[Lldb-commits] [lldb] c2e9677 - [lldb-dap] Ensure we do not print the close sentinel when closing stdout. (#126833)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Feb 13 10:35:54 PST 2025
Author: John Harrison
Date: 2025-02-13T10:35:50-08:00
New Revision: c2e96778e04197dd266f7c540bf174b6ec28a434
URL: https://github.com/llvm/llvm-project/commit/c2e96778e04197dd266f7c540bf174b6ec28a434
DIFF: https://github.com/llvm/llvm-project/commit/c2e96778e04197dd266f7c540bf174b6ec28a434.diff
LOG: [lldb-dap] Ensure we do not print the close sentinel when closing stdout. (#126833)
If you have an lldb-dap log file you'll almost always see a final
message like:
```
<--
Content-Length: 94
{
"body": {
"category": "stdout",
"output": "\u0000\u0000"
},
"event": "output",
"seq": 0,
"type": "event"
}
<--
Content-Length: 94
{
"body": {
"category": "stderr",
"output": "\u0000\u0000"
},
"event": "output",
"seq": 0,
"type": "event"
}
```
The OutputRedirect is always writing the `"\0"` byte as a final stdout
message during shutdown. Instead, I adjusted this to detect the sentinel
value and break out of the read loop as if we detected EOF.
---------
Co-authored-by: Pavel Labath <pavel at labath.sk>
Added:
Modified:
lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
lldb/test/API/tools/lldb-dap/output/main.c
lldb/tools/lldb-dap/OutputRedirector.cpp
lldb/tools/lldb-dap/OutputRedirector.h
Removed:
################################################################################
diff --git a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
index 02c34ba10321b..eae7629409844 100644
--- a/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
+++ b/lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
@@ -10,23 +10,33 @@
class TestDAP_output(lldbdap_testcase.DAPTestCaseBase):
@skipIfWindows
def test_output(self):
+ """
+ Test output handling for the running process.
+ """
program = self.getBuildArtifact("a.out")
- self.build_and_launch(program)
+ self.build_and_launch(
+ program,
+ exitCommands=[
+ # Ensure that output produced by lldb itself is not consumed by the OutputRedirector.
+ "?script print('out\\0\\0', end='\\r\\n', file=sys.stdout)",
+ "?script print('err\\0\\0', end='\\r\\n', file=sys.stderr)",
+ ],
+ )
source = "main.c"
lines = [line_number(source, "// breakpoint 1")]
breakpoint_ids = self.set_source_breakpoints(source, lines)
self.continue_to_breakpoints(breakpoint_ids)
-
+
# Ensure partial messages are still sent.
output = self.collect_stdout(timeout_secs=1.0, pattern="abcdef")
- self.assertTrue(output and len(output) > 0, "expect no program output")
+ self.assertTrue(output and len(output) > 0, "expect program stdout")
self.continue_to_exit()
-
+
output += self.get_stdout(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval)
- self.assertTrue(output and len(output) > 0, "expect no program output")
+ self.assertTrue(output and len(output) > 0, "expect program stdout")
self.assertIn(
- "abcdefghi\r\nhello world\r\n",
+ "abcdefghi\r\nhello world\r\nfinally\0\0out\0\0\r\nerr\0\0",
output,
- 'full output not found in: ' + output,
+ "full stdout not found in: " + repr(output),
)
diff --git a/lldb/test/API/tools/lldb-dap/output/main.c b/lldb/test/API/tools/lldb-dap/output/main.c
index 0cfcf604aa68f..226cb607a1234 100644
--- a/lldb/test/API/tools/lldb-dap/output/main.c
+++ b/lldb/test/API/tools/lldb-dap/output/main.c
@@ -8,5 +8,8 @@ int main() {
printf("def");
printf("ghi\n");
printf("hello world\n"); // breakpoint 1
+ // Ensure the OutputRedirector does not consume the programs \0\0 output.
+ char buf[] = "finally\0";
+ write(STDOUT_FILENO, buf, sizeof(buf));
return 0;
}
diff --git a/lldb/tools/lldb-dap/OutputRedirector.cpp b/lldb/tools/lldb-dap/OutputRedirector.cpp
index a23572ab7ae04..df21fb850dc55 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.cpp
+++ b/lldb/tools/lldb-dap/OutputRedirector.cpp
@@ -10,6 +10,7 @@
#include "DAP.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
+#include <cstring>
#include <system_error>
#if defined(_WIN32)
#include <fcntl.h>
@@ -18,12 +19,9 @@
#include <unistd.h>
#endif
-using lldb_private::Pipe;
-using llvm::createStringError;
-using llvm::Error;
-using llvm::Expected;
-using llvm::inconvertibleErrorCode;
-using llvm::StringRef;
+using namespace llvm;
+
+static constexpr auto kCloseSentinel = StringLiteral::withInnerNUL("\0");
namespace lldb_dap {
@@ -58,9 +56,6 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
char buffer[OutputBufferSize];
while (!m_stopped) {
ssize_t bytes_count = ::read(read_fd, &buffer, sizeof(buffer));
- // EOF detected.
- if (bytes_count == 0)
- break;
if (bytes_count == -1) {
// Skip non-fatal errors.
if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK)
@@ -68,7 +63,13 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
break;
}
- callback(StringRef(buffer, bytes_count));
+ StringRef data(buffer, bytes_count);
+ if (m_stopped)
+ data.consume_back(kCloseSentinel);
+ if (data.empty())
+ break;
+
+ callback(data);
}
::close(read_fd);
});
@@ -85,8 +86,7 @@ void OutputRedirector::Stop() {
// Closing the pipe may not be sufficient to wake up the thread in case the
// write descriptor is duplicated (to stdout/err or to another process).
// Write a null byte to ensure the read call returns.
- char buf[] = "\0";
- (void)::write(fd, buf, sizeof(buf));
+ (void)::write(fd, kCloseSentinel.data(), kCloseSentinel.size());
::close(fd);
m_forwarder.join();
}
diff --git a/lldb/tools/lldb-dap/OutputRedirector.h b/lldb/tools/lldb-dap/OutputRedirector.h
index d2bd39797f3d7..a47ea96f71f14 100644
--- a/lldb/tools/lldb-dap/OutputRedirector.h
+++ b/lldb/tools/lldb-dap/OutputRedirector.h
@@ -9,7 +9,6 @@
#ifndef LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
#define LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
-#include "lldb/Host/Pipe.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <atomic>
More information about the lldb-commits
mailing list