Adding a test with a zero-byte write and ensuring we only append to the buffer if we have data to append.

Author: ashgti

lldb/include/lldb/Host/JSONTransport.h

@@ -110,20 +110,24 @@ class JSONTransport {
             return;
           }
 
-          m_buffer.append(std::string(buffer, len));
-
-          llvm::Expected<std::vector<std::string>> messages = Parse();
-          if (llvm::Error error = messages.takeError()) {
-            callback(loop, std::move(error));
-            return;
+          if (len)
+            m_buffer.append(std::string(buffer, len));
+
+          // If the buffer has contents, try parsing any pending messages.
+          if (!m_buffer.empty()) {
+            llvm::Expected<std::vector<std::string>> messages = Parse();
+            if (llvm::Error error = messages.takeError()) {
+              callback(loop, std::move(error));
+              return;
+            }
+
+            for (const auto &message : *messages)
+              if constexpr (std::is_same<T, std::string>::value)
+                callback(loop, message);
+              else
+                callback(loop, llvm::json::parse<T>(message));
           }
 
-          for (const auto &message : *messages)
-            if constexpr (std::is_same<T, std::string>::value)
-              callback(loop, message);
-            else
-              callback(loop, llvm::json::parse<T>(message));
-
           // On EOF, notify the callback after the remaining messages were
           // handled.
           if (len == 0) {

lldb/tools/lldb-dap/DAP.cpp

@@ -1004,10 +1004,8 @@ Status DAP::TransportHandler() {
           }
         }
 
-        {
-          std::lock_guard<std::mutex> guard(m_queue_mutex);
-          m_queue.push_back(std::move(*message));
-        }
+        std::lock_guard<std::mutex> guard(m_queue_mutex);
+        m_queue.push_back(std::move(*message));
         m_queue_cv.notify_one();
       });
   if (auto err = handle.takeError())

lldb/unittests/DAP/TestBase.h

@@ -28,9 +28,8 @@ class TransportBase : public PipePairTest {
   void SetUp() override;
 
   template <typename P>
-  void
-  RunOnce(std::function<void(llvm::Expected<P>)> callback,
-          std::chrono::milliseconds timeout = std::chrono::milliseconds(100)) {
+  void RunOnce(std::function<void(llvm::Expected<P>)> callback,
+               std::chrono::milliseconds timeout = std::chrono::seconds(1)) {
     auto handle = from_dap->RegisterReadObject<P>(
         loop, [&](lldb_private::MainLoopBase &loop, llvm::Expected<P> message) {
           callback(std::move(message));

lldb/unittests/Host/JSONTransportTest.cpp

@@ -173,6 +173,25 @@ TEST_F(HTTPDelimitedJSONTransportTest, ReadPartialMessage) {
                        HasValue(testing::FieldsAre(/*str=*/"foo")));
 }
 
+TEST_F(HTTPDelimitedJSONTransportTest, ReadWithZeroByteWrites) {
+  std::future<void> background_task = std::async(std::launch::async, [&]() {
+    std::string json = R"({"str": "foo"})";
+    std::string message =
+        formatv("Content-Length: {0}\r\n\r\n{1}", json.size(), json).str();
+    std::string part1 = message.substr(0, 28);
+    std::string part2 = message.substr(28);
+
+    ASSERT_THAT_EXPECTED(input.Write(part1.data(), part1.size()), Succeeded());
+    ASSERT_THAT_EXPECTED(input.Write(part1.data(), 0),
+                         Succeeded()); // zero-byte write.
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    ASSERT_THAT_EXPECTED(input.Write(part2.data(), part2.size()), Succeeded());
+  });
+
+  ASSERT_THAT_EXPECTED(RunOnce<JSONTestType>(),
+                       HasValue(testing::FieldsAre(/*str=*/"foo")));
+}
+
 TEST_F(HTTPDelimitedJSONTransportTest, ReadWithEOF) {
   input.CloseWriteFileDescriptor();
   ASSERT_THAT_EXPECTED(RunOnce<JSONTestType>(), Failed<TransportEOFError>());