Skip to content

Commit c2e9677

Browse files
ashgtilabath
andauthored
[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 <[email protected]>
1 parent ea77dd8 commit c2e9677

File tree

4 files changed

+32
-20
lines changed

4 files changed

+32
-20
lines changed

lldb/test/API/tools/lldb-dap/output/TestDAP_output.py

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,33 @@
1010
class TestDAP_output(lldbdap_testcase.DAPTestCaseBase):
1111
@skipIfWindows
1212
def test_output(self):
13+
"""
14+
Test output handling for the running process.
15+
"""
1316
program = self.getBuildArtifact("a.out")
14-
self.build_and_launch(program)
17+
self.build_and_launch(
18+
program,
19+
exitCommands=[
20+
# Ensure that output produced by lldb itself is not consumed by the OutputRedirector.
21+
"?script print('out\\0\\0', end='\\r\\n', file=sys.stdout)",
22+
"?script print('err\\0\\0', end='\\r\\n', file=sys.stderr)",
23+
],
24+
)
1525
source = "main.c"
1626
lines = [line_number(source, "// breakpoint 1")]
1727
breakpoint_ids = self.set_source_breakpoints(source, lines)
1828
self.continue_to_breakpoints(breakpoint_ids)
19-
29+
2030
# Ensure partial messages are still sent.
2131
output = self.collect_stdout(timeout_secs=1.0, pattern="abcdef")
22-
self.assertTrue(output and len(output) > 0, "expect no program output")
32+
self.assertTrue(output and len(output) > 0, "expect program stdout")
2333

2434
self.continue_to_exit()
25-
35+
2636
output += self.get_stdout(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval)
27-
self.assertTrue(output and len(output) > 0, "expect no program output")
37+
self.assertTrue(output and len(output) > 0, "expect program stdout")
2838
self.assertIn(
29-
"abcdefghi\r\nhello world\r\n",
39+
"abcdefghi\r\nhello world\r\nfinally\0\0out\0\0\r\nerr\0\0",
3040
output,
31-
'full output not found in: ' + output,
41+
"full stdout not found in: " + repr(output),
3242
)

lldb/test/API/tools/lldb-dap/output/main.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,8 @@ int main() {
88
printf("def");
99
printf("ghi\n");
1010
printf("hello world\n"); // breakpoint 1
11+
// Ensure the OutputRedirector does not consume the programs \0\0 output.
12+
char buf[] = "finally\0";
13+
write(STDOUT_FILENO, buf, sizeof(buf));
1114
return 0;
1215
}

lldb/tools/lldb-dap/OutputRedirector.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "DAP.h"
1111
#include "llvm/ADT/StringRef.h"
1212
#include "llvm/Support/Error.h"
13+
#include <cstring>
1314
#include <system_error>
1415
#if defined(_WIN32)
1516
#include <fcntl.h>
@@ -18,12 +19,9 @@
1819
#include <unistd.h>
1920
#endif
2021

21-
using lldb_private::Pipe;
22-
using llvm::createStringError;
23-
using llvm::Error;
24-
using llvm::Expected;
25-
using llvm::inconvertibleErrorCode;
26-
using llvm::StringRef;
22+
using namespace llvm;
23+
24+
static constexpr auto kCloseSentinel = StringLiteral::withInnerNUL("\0");
2725

2826
namespace lldb_dap {
2927

@@ -58,17 +56,20 @@ Error OutputRedirector::RedirectTo(std::function<void(StringRef)> callback) {
5856
char buffer[OutputBufferSize];
5957
while (!m_stopped) {
6058
ssize_t bytes_count = ::read(read_fd, &buffer, sizeof(buffer));
61-
// EOF detected.
62-
if (bytes_count == 0)
63-
break;
6459
if (bytes_count == -1) {
6560
// Skip non-fatal errors.
6661
if (errno == EAGAIN || errno == EINTR || errno == EWOULDBLOCK)
6762
continue;
6863
break;
6964
}
7065

71-
callback(StringRef(buffer, bytes_count));
66+
StringRef data(buffer, bytes_count);
67+
if (m_stopped)
68+
data.consume_back(kCloseSentinel);
69+
if (data.empty())
70+
break;
71+
72+
callback(data);
7273
}
7374
::close(read_fd);
7475
});
@@ -85,8 +86,7 @@ void OutputRedirector::Stop() {
8586
// Closing the pipe may not be sufficient to wake up the thread in case the
8687
// write descriptor is duplicated (to stdout/err or to another process).
8788
// Write a null byte to ensure the read call returns.
88-
char buf[] = "\0";
89-
(void)::write(fd, buf, sizeof(buf));
89+
(void)::write(fd, kCloseSentinel.data(), kCloseSentinel.size());
9090
::close(fd);
9191
m_forwarder.join();
9292
}

lldb/tools/lldb-dap/OutputRedirector.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#ifndef LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
1010
#define LLDB_TOOLS_LLDB_DAP_OUTPUT_REDIRECTOR_H
1111

12-
#include "lldb/Host/Pipe.h"
1312
#include "llvm/ADT/StringRef.h"
1413
#include "llvm/Support/Error.h"
1514
#include <atomic>

0 commit comments

Comments
 (0)