Handle EOF more gracefully in DAP

Message ID 20240110200900.791199-1-tromey@adacore.com
State New
Headers
Series Handle EOF more gracefully in DAP |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Jan. 10, 2024, 8:09 p.m. UTC
  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
---
 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(-)
 create mode 100644 gdb/testsuite/gdb.dap/eof.exp
  

Comments

Tom Tromey Jan. 22, 2024, 3:47 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> A user pointed out that gdb will print a Python exception when it gets
Tom> an EOF in DAP mode.  And, it turns out that an EOF like this also
Tom> causes gdb not to exit.  This is due to the refactoring that moved the
Tom> JSON reader to its own thread -- previously this caused an exception
Tom> to propagate and cause an exit, but now it just leaves the reader
Tom> hung.

Tom> This patch fixes these problems by arranging to handle EOF more
Tom> gracefully.

Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31217

I'm checking this in.

Tom
  

Patch

diff --git a/gdb/python/lib/gdb/dap/io.py b/gdb/python/lib/gdb/dap/io.py
index 2f3a2351925..367af481086 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 59fad59d8e6..17a41564a92 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 bafcd9b6fc7..72b6ac537c3 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