[binutils-gdb] Handle EOF more gracefully in DAP

[binutils-gdb] Handle EOF more gracefully in DAP

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=46bdb641642d20307521f5db2e1f1c7dcc49d5a7

commit 46bdb641642d20307521f5db2e1f1c7dcc49d5a7
Author: Tom Tromey <tromey@adacore.com>
Date:   Wed Jan 10 12:51:10 2024 -0700

    Handle EOF more gracefully in DAP
    
    A user pointed out that gdb will print a Python exception when it gets
    an EOF in DAP mode.  And, it turns out that an EOF like this also
    causes gdb not to exit.  This is due to the refactoring that moved the
    JSON reader to its own thread -- previously this caused an exception
    to propagate and cause an exit, but now it just leaves the reader
    hung.
    
    This patch fixes these problems by arranging to handle EOF more
    gracefully.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31217

Diff:
---
 gdb/python/lib/gdb/dap/io.py      | 54 ++++++++++++++++++++++++---------------
 gdb/python/lib/gdb/dap/server.py  |  7 +++++
 gdb/testsuite/gdb.dap/eof.exp     | 37 +++++++++++++++++++++++++++
 gdb/testsuite/lib/dap-support.exp |  4 +--
 4 files changed, 80 insertions(+), 22 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/io.py b/gdb/python/lib/gdb/dap/io.py
index 611f0c2ad09..5149edae977 100644
--- a/gdb/python/lib/gdb/dap/io.py
+++ b/gdb/python/lib/gdb/dap/io.py
@@ -15,30 +15,44 @@
 
 import json
 
-from .startup import start_thread, send_gdb, log
+from .startup import start_thread, send_gdb, log, log_stack, LogLevel
 
 
 def read_json(stream):
     """Read a JSON-RPC message from STREAM.
-    The decoded object is returned."""
-    # First read and parse the header.
-    content_length = None
-    while True:
-        line = stream.readline()
-        line = line.strip()
-        if line == b"":
-            break
-        if line.startswith(b"Content-Length:"):
-            line = line[15:].strip()
-            content_length = int(line)
-            continue
-        log("IGNORED: <<<%s>>>" % line)
-    data = bytes()
-    while len(data) < content_length:
-        new_data = stream.read(content_length - len(data))
-        data += new_data
-    result = json.loads(data)
-    return result
+    The decoded object is returned.
+    None is returned on EOF."""
+    try:
+        # First read and parse the header.
+        content_length = None
+        while True:
+            line = stream.readline()
+            # If the line is empty, we hit EOF.
+            if len(line) == 0:
+                log("EOF")
+                return None
+            line = line.strip()
+            if line == b"":
+                break
+            if line.startswith(b"Content-Length:"):
+                line = line[15:].strip()
+                content_length = int(line)
+                continue
+            log("IGNORED: <<<%s>>>" % line)
+        data = bytes()
+        while len(data) < content_length:
+            new_data = stream.read(content_length - len(data))
+            # Maybe we hit EOF.
+            if len(new_data) == 0:
+                log("EOF after reading the header")
+                return None
+            data += new_data
+        return json.loads(data)
+    except OSError:
+        # Reading can also possibly throw an exception.  Treat this as
+        # EOF.
+        log_stack(LogLevel.FULL)
+        return None
 
 
 def start_json_writer(stream, queue):
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index e96c79dc736..7cc5a4681ee 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -187,6 +187,8 @@ class Server:
     def _reader_thread(self):
         while True:
             cmd = read_json(self.in_stream)
+            if cmd is None:
+                break
             log("READ: <<<" + json.dumps(cmd) + ">>>")
             # Be extra paranoid about the form here.  If anything is
             # missing, it will be put in the queue and then an error
@@ -201,6 +203,8 @@ class Server:
             ):
                 self.canceller.cancel(cmd["arguments"]["requestId"])
             self.read_queue.put(cmd)
+        # When we hit EOF, signal it with None.
+        self.read_queue.put(None)
 
     @in_dap_thread
     def main_loop(self):
@@ -212,6 +216,9 @@ class Server:
         start_thread("JSON reader", self._reader_thread)
         while not self.done:
             cmd = self.read_queue.get()
+            # A None value here means the reader hit EOF.
+            if cmd is None:
+                break
             result = self._handle_command(cmd)
             self._send_json(result)
             events = self.delayed_events
diff --git a/gdb/testsuite/gdb.dap/eof.exp b/gdb/testsuite/gdb.dap/eof.exp
new file mode 100644
index 00000000000..139c17ad335
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/eof.exp
@@ -0,0 +1,37 @@
+# Copyright 2024 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that EOF is handled gracefully.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+# The test doesn't matter much.
+standard_testfile scopes.c
+
+if {[build_executable ${testfile}.exp $testfile $srcfile] == -1} {
+    return
+}
+
+if {[dap_launch $testfile] == ""} {
+    return
+}
+
+catch "close -i $gdb_spawn_id"
+catch "wait -i $gdb_spawn_id"
+unset gdb_spawn_id
+
+dap_check_log_file
diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index a0c6051cdc7..0bb413e4037 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -339,7 +339,7 @@ proc dap_target_remote {target} {
 }
 
 # Read the most recent DAP log file and check it for exceptions.
-proc _dap_check_log_file {} {
+proc dap_check_log_file {} {
     global dap_gdb_instance
 
     set fd [open [standard_output_file "dap.log.$dap_gdb_instance"]]
@@ -366,7 +366,7 @@ proc _dap_check_log_file {} {
 proc dap_shutdown {{terminate false}} {
     dap_check_request_and_response "shutdown" disconnect \
 	[format {o terminateDebuggee [l %s]} $terminate]
-    _dap_check_log_file
+    dap_check_log_file
 }
 
 # Search the event list EVENTS for an output event matching the regexp