Skip to content
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
72 changes: 72 additions & 0 deletions lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
"""
Test lldb-dap IO handling.
"""

from lldbsuite.test.decorators import *
import lldbdap_testcase
import dap_server


class TestDAP_io(lldbdap_testcase.DAPTestCaseBase):
def launch(self):
log_file_path = self.getBuildArtifact("dap.txt")
process, _ = dap_server.DebugAdapterServer.launch(
executable=self.lldbDAPExec, log_file=log_file_path
)

def cleanup():
# If the process is still alive, terminate it.
if process.poll() is None:
process.terminate()
stdout_data = process.stdout.read()
stderr_data = process.stderr.read()
print("========= STDOUT =========")
print(stdout_data)
print("========= END =========")
print("========= STDERR =========")
print(stderr_data)
print("========= END =========")
print("========= DEBUG ADAPTER PROTOCOL LOGS =========")
with open(log_file_path, "r") as file:
print(file.read())
print("========= END =========")

# Execute the cleanup function during test case tear down.
self.addTearDownHook(cleanup)

return process

def test_eof_immediately(self):
"""
lldb-dap handles EOF without any other input.
"""
process = self.launch()
process.stdin.close()
self.assertEqual(process.wait(timeout=5.0), 0)

def test_partial_header(self):
"""
lldb-dap handles parital message headers.
"""
process = self.launch()
process.stdin.write(b"Content-Length: ")
process.stdin.close()
self.assertEqual(process.wait(timeout=5.0), 0)

def test_incorrect_content_length(self):
"""
lldb-dap handles malformed content length headers.
"""
process = self.launch()
process.stdin.write(b"Content-Length: abc")
process.stdin.close()
self.assertEqual(process.wait(timeout=5.0), 0)

def test_partial_content_length(self):
"""
lldb-dap handles partial messages.
"""
process = self.launch()
process.stdin.write(b"Content-Length: 10{")
process.stdin.close()
self.assertEqual(process.wait(timeout=5.0), 0)
6 changes: 5 additions & 1 deletion lldb/tools/lldb-dap/DAP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,11 @@ lldb::SBError DAP::Disconnect(bool terminateDebuggee) {
}

llvm::Error DAP::Loop() {
auto cleanup = llvm::make_scope_exit([this]() { StopEventHandlers(); });
auto cleanup = llvm::make_scope_exit([this]() {
if (output.descriptor)
output.descriptor->Close();
Comment on lines +860 to +861
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little worried about the lack of abstraction here. Should {Input,Output}Stream be a class instead of a struct and encapsulate this operation? We close the descriptor (if it's owned) when we destroy the IOObjectSP. So should we just reset the IOObjectSP pointer instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to get rid of the InputStream and OutputStream classes altogether. They seem to add very little on top of IOObjectSP so maybe we can just store those directly, or alternative, make the relation an "is a" instead of a "has a".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a patch to do just that in ashgti@a57a16f but I was waiting to do that until after I submitted #129155

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wasn't sure if I should submit the logging refactor because of #129294 (comment) so that is another PR I was waiting on figuring out first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah fair enough, let me take another look at that PR.

StopEventHandlers();
});
while (!disconnecting) {
llvm::json::Object object;
lldb_dap::PacketStatus status = GetNextObject(object);
Expand Down
12 changes: 10 additions & 2 deletions lldb/tools/lldb-dap/IOStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,23 @@ bool InputStream::read_full(std::ofstream *log, size_t length,
if (status.Fail())
return false;

text += data;
text += data.substr(0, length);
return true;
}

bool InputStream::read_line(std::ofstream *log, std::string &line) {
line.clear();
while (true) {
if (!read_full(log, 1, line))
std::string next;
if (!read_full(log, 1, next))
return false;

// If EOF is encoutnered, '' is returned, break out of this loop.
if (next.empty())
return false;

line += next;

if (llvm::StringRef(line).ends_with("\r\n"))
break;
}
Expand All @@ -60,6 +67,7 @@ bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
if (log)
*log << "Warning: Expected '" << expected.str() << "', got '" << result
<< "\n";
return false;
}
return true;
}