public inbox for gdb-prs@sourceware.org
help / color / mirror / Atom feed
* [Bug dap/31380] New: [gdb/dap] Ensure responses are flushed to client before exiting
@ 2024-02-14 11:23 vries at gcc dot gnu.org
  2024-02-15 18:19 ` [Bug dap/31380] " tromey at sourceware dot org
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: vries at gcc dot gnu.org @ 2024-02-14 11:23 UTC (permalink / raw)
  To: gdb-prs

https://sourceware.org/bugzilla/show_bug.cgi?id=31380

            Bug ID: 31380
           Summary: [gdb/dap] Ensure responses are flushed to client
                    before exiting
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: dap
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

In dap, we want to flush responses to the client before exiting, as stated here
in main_loop:
...
        # Got the terminate request.  This is handled by the                    
        # JSON-writing thread, so that we can ensure that all                   
        # responses are flushed to the client before exiting.                   
        self.write_queue.put(None)
...

Now that we print "<dap thread name>: terminating" message, we can check for
this (by proxy) by doing:
...
diff --git a/gdb/testsuite/gdb.dap/eof.exp b/gdb/testsuite/gdb.dap/eof.exp
index 9c17725c0d0..575a98d6042 100644
--- a/gdb/testsuite/gdb.dap/eof.exp
+++ b/gdb/testsuite/gdb.dap/eof.exp
@@ -35,3 +35,4 @@ catch "wait -i $gdb_spawn_id"
 unset gdb_spawn_id

 dap_check_log_file
+dap_check_log_file_re "JSON writer: terminating"
diff --git a/gdb/testsuite/lib/dap-support.exp
b/gdb/testsuite/lib/dap-support.exp
index 979dfa2cd73..b56d8bee244 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -358,6 +358,17 @@ proc dap_check_log_file {} {
     }
 }

+proc dap_check_log_file_re { re } {
+    global dap_gdb_instance
+
+    set fd [open [standard_output_file "dap.log.$dap_gdb_instance"]]
+    set contents [read $fd]
+    close $fd
+
+    set ok [regexp $re $contents]
+    gdb_assert {$ok} "log file matched $re"
+}
+
 # Cleanly shut down gdb.  TERMINATE is passed as the terminateDebuggee
 # parameter to the request.
 proc dap_shutdown {{terminate false}} {
...

This currently fails more often than not:
...
FAIL: gdb.dap/eof.exp: log file matched JSON writer: terminating
...

This ( https://sourceware.org/pipermail/gdb-patches/2024-February/206416.html )
mentions:
...
However, I suppose this approach does not really work, because main_loop then
proceeds to return.

One way to fix this would be to have start_json_writer return the thread
object, and then have main_loop join the thread to terminate after
writing the None.  This would mean making this particular thread
non-daemon though.
...

I posted a first attempt at fixing this here (
https://sourceware.org/pipermail/gdb-patches/2024-February/206568.html ):
...
diff --git a/gdb/python/lib/gdb/dap/io.py b/gdb/python/lib/gdb/dap/io.py
index 5149edae977..491cf208ae3 100644
--- a/gdb/python/lib/gdb/dap/io.py
+++ b/gdb/python/lib/gdb/dap/io.py
@@ -65,10 +65,7 @@ def start_json_writer(stream, queue):
         while True:
             obj = queue.get()
             if obj is None:
-                # This is an exit request.  The stream is already
-                # flushed, so all that's left to do is request an
-                # exit.
-                send_gdb("quit")
+                queue.task_done()
                 break
             obj["seq"] = seq
             seq = seq + 1
@@ -79,5 +76,6 @@ def start_json_writer(stream, queue):
             stream.write(header_bytes)
             stream.write(body_bytes)
             stream.flush()
+            queue.task_done()

     start_thread("JSON writer", _json_writer)
diff --git a/gdb/python/lib/gdb/dap/server.py
b/gdb/python/lib/gdb/dap/server.py
index 7cc5a4681ee..ca4465ff207 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -229,6 +229,8 @@ class Server:
         # JSON-writing thread, so that we can ensure that all
         # responses are flushed to the client before exiting.
         self.write_queue.put(None)
+        self.write_queue.join()
+        send_gdb("quit")

     @in_dap_thread
     def send_event_later(self, event, body=None):
...

But I still run into the same fail.

All that this patch does is guarantee that the dap main thread doesn't finish
before the JSON writer thread.

But there's a race with the SIGHUP that's handled by GDB's main thread.  AFAIU
there's nothing guaranteeing that the dap main thread finishes before GDB's
main thread.

If we're going to add such a constraint, it could make sense to stop dap
earlier, before GDB's main thread looses the ability to handle posted (python)
events.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-22 10:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-14 11:23 [Bug dap/31380] New: [gdb/dap] Ensure responses are flushed to client before exiting vries at gcc dot gnu.org
2024-02-15 18:19 ` [Bug dap/31380] " tromey at sourceware dot org
2024-02-15 18:21 ` tromey at sourceware dot org
2024-02-16 11:46 ` vries at gcc dot gnu.org
2024-02-19  8:25 ` vries at gcc dot gnu.org
2024-02-19  8:43 ` vries at gcc dot gnu.org
2024-02-21  9:46 ` cvs-commit at gcc dot gnu.org
2024-02-22 10:35 ` cvs-commit at gcc dot gnu.org
2024-02-22 10:40 ` vries at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).