Skip to content

[lldb-dap] Ensure we do not print the close sentinel when closing stdout. #126833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions lldb/test/API/tools/lldb-dap/output/TestDAP_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
3 changes: 3 additions & 0 deletions lldb/test/API/tools/lldb-dap/output/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
24 changes: 12 additions & 12 deletions lldb/tools/lldb-dap/OutputRedirector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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 {

Expand Down Expand Up @@ -58,17 +56,20 @@ 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)
continue;
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);
});
Expand All @@ -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();
}
Expand Down
1 change: 0 additions & 1 deletion lldb/tools/lldb-dap/OutputRedirector.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down