public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] Implement DAP cancellation
@ 2023-12-11 16:02 Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 1/6] Clean up handling of DAP not-stopped response Tom Tromey
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Kévin Le Gouguec, Eli Zaretskii

DAP specifies a way to cancel requests.  Previously, I didn't
implement this, because it didn't seem necessary.  However, I realized
later that a 'repl' evaluation can cause the inferior to continue (or
gdb to invoke a long-running CLI or Python command), and a request
like this will not finish -- cancellation is necessary.

This series implements this feature.  I've split it into bite-sized
pieces.

Tested on x86-64 Fedora 38.

---
Changes in v2:
- Updated documentation per review
- Link to v1: https://inbox.sourceware.org/gdb-patches/20231201-dap-cancel-v1-0-872022fc328a@adacore.com

---
Tom Tromey (6):
      Clean up handling of DAP not-stopped response
      Move DAP JSON reader to its own thread
      Introduce gdb.interrupt
      Rename a couple of DAP procs in the testsuite
      Catch KeyboardInterrupt in send_gdb_with_response
      Implement DAP cancellation

 gdb/NEWS                          |   5 ++
 gdb/doc/gdb.texinfo               |  16 ++++++
 gdb/doc/python.texi               |  11 ++++
 gdb/python/lib/gdb/dap/server.py  | 115 ++++++++++++++++++++++++++++++++++++--
 gdb/python/lib/gdb/dap/startup.py |   4 +-
 gdb/python/python.c               |  19 +++++++
 gdb/testsuite/gdb.dap/pause.exp   |  71 +++++++++++++++++++++++
 gdb/testsuite/lib/dap-support.exp |  10 ++--
 8 files changed, 239 insertions(+), 12 deletions(-)
---
base-commit: 9a099bc220fbc97e9ccff6a7e1861e11665e8899
change-id: 20231201-dap-cancel-2c292d80ba37

Best regards,
-- 
Tom Tromey <tromey@adacore.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/6] Clean up handling of DAP not-stopped response
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
@ 2023-12-11 16:02 ` Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 2/6] Move DAP JSON reader to its own thread Tom Tromey
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Kévin Le Gouguec

This patch introduces a new NotStoppedException type and changes the
DAP implementation of "not stopped" to use it.  I was already touching
some code in this area and I thought this looked a little cleaner.
This also has the advantage that we can now choose not to log the
exception -- previously I was sometimes a bit alarmed when seeing this
in the logs, even though it is harmless.

Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
---
 gdb/python/lib/gdb/dap/server.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index ead911d11da..d865dee31a8 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -42,6 +42,12 @@ _commands = {}
 _server = None
 
 
+# A subclass of Exception that is used solely for reporting that a
+# request needs the inferior to be stopped, but it is not stopped.
+class NotStoppedException(Exception):
+    pass
+
+
 class Server:
     """The DAP server class."""
 
@@ -78,6 +84,9 @@ class Server:
             if body is not None:
                 result["body"] = body
             result["success"] = True
+        except NotStoppedException:
+            result["success"] = False
+            result["message"] = "notStopped"
         except BaseException as e:
             log_stack()
             result["success"] = False
@@ -169,7 +178,7 @@ def _check_not_running(func):
         from .events import inferior_running
 
         if inferior_running:
-            raise Exception("notStopped")
+            raise NotStoppedException()
         return func(*args, **kwargs)
 
     return check

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/6] Move DAP JSON reader to its own thread
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 1/6] Clean up handling of DAP not-stopped response Tom Tromey
@ 2023-12-11 16:02 ` Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 3/6] Introduce gdb.interrupt Tom Tromey
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Kévin Le Gouguec

This changes the DAP server to move the JSON reader to a new thread.
This is key to implementing request cancellation, as now requests can
be read while an earlier one is being serviced.

Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
---
 gdb/python/lib/gdb/dap/server.py | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index d865dee31a8..53a0ca7f448 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -60,6 +60,9 @@ class Server:
         # DAP client.  Writing is done in a separate thread to avoid
         # blocking the read loop.
         self.write_queue = DAPQueue()
+        # Reading is also done in a separate thread, and a queue of
+        # requests is kept.
+        self.read_queue = DAPQueue()
         self.done = False
         global _server
         _server = self
@@ -111,6 +114,14 @@ class Server:
         log("WROTE: <<<" + json.dumps(obj) + ">>>")
         self.write_queue.put(obj)
 
+    # This is run in a separate thread and simply reads requests from
+    # the client and puts them into a queue.
+    def _reader_thread(self):
+        while True:
+            cmd = read_json(self.in_stream)
+            log("READ: <<<" + json.dumps(cmd) + ">>>")
+            self.read_queue.put(cmd)
+
     @in_dap_thread
     def main_loop(self):
         """The main loop of the DAP server."""
@@ -118,9 +129,9 @@ class Server:
         # client, and the thread that reads output from the inferior.
         start_thread("output reader", self._read_inferior_output)
         start_json_writer(self.out_stream, self.write_queue)
+        start_thread("JSON reader", self._reader_thread)
         while not self.done:
-            cmd = read_json(self.in_stream)
-            log("READ: <<<" + json.dumps(cmd) + ">>>")
+            cmd = self.read_queue.get()
             result = self._handle_command(cmd)
             self._send_json(result)
             events = self.delayed_events

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 3/6] Introduce gdb.interrupt
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 1/6] Clean up handling of DAP not-stopped response Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 2/6] Move DAP JSON reader to its own thread Tom Tromey
@ 2023-12-11 16:02 ` Tom Tromey
  2023-12-11 16:30   ` Eli Zaretskii
  2023-12-11 16:02 ` [PATCH v2 4/6] Rename a couple of DAP procs in the testsuite Tom Tromey
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Eli Zaretskii, Kévin Le Gouguec

DAP cancellation needs a way to interrupt whatever is happening on
gdb's main thread -- whether that is the inferior, a gdb CLI command,
or Python code.

This patch adds a new gdb.interrupt() function for this purpose.  It
simply sets the quit flag and lets gdb do the rest.

No tests in this patch -- instead this is tested via the DAP
cancellation tests.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
---
 gdb/NEWS            |  3 +++
 gdb/doc/python.texi | 11 +++++++++++
 gdb/python/python.c | 19 +++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index aaf8faad792..900ac47ada9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -75,6 +75,9 @@ show remote thread-options-packet
      sub-classed to create handlers for objfiles with missing debug
      information.
 
+  ** New function gdb.interrupt(), that interrupts GDB as if the user
+     typed control-c.
+
 * Debugger Adapter Protocol changes
 
   ** GDB now emits the "process" event.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 3a35c5c2ccc..ede8b1fb6b5 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -664,6 +664,17 @@ this an easy-to-use drop-in replacement for creating threads that will
 work well in @value{GDBN}.
 @end deftp
 
+@defun gdb.interrupt ()
+This causes @value{GDBN} to react as if the user had typed a control-C
+character at the terminal.  That is, if the inferior is running, it is
+interrupted; if a @value{GDBN} command is executing, it is stopped;
+and if a Python command is running, @code{KeyboardInterrupt} will be
+raised.
+
+Unlike most Python APIs in @value{GDBN}, @code{interrupt} is
+thread-safe.
+@end defun
+
 @defun gdb.post_event (event)
 Put @var{event}, a callable object taking no arguments, into
 @value{GDBN}'s internal event queue.  This callable will be invoked at
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8a36673a3e4..832f374b39f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1118,6 +1118,23 @@ gdbpy_post_event (PyObject *self, PyObject *args)
   Py_RETURN_NONE;
 }
 
+/* Interrupt the current operation on the main thread.  */
+static PyObject *
+gdbpy_interrupt (PyObject *self, PyObject *args)
+{
+  {
+    /* Make sure the interrupt isn't delivered immediately somehow.
+       This probably is not truly needed, but at the same time it
+       seems more clear to be explicit about the intent.  */
+    gdbpy_allow_threads temporarily_exit_python;
+    scoped_disable_cooperative_sigint_handling no_python_sigint;
+
+    set_quit_flag ();
+  }
+
+  Py_RETURN_NONE;
+}
+
 \f
 
 /* This is the extension_language_ops.before_prompt "method".  */
@@ -2678,6 +2695,8 @@ Parse String as an expression, evaluate it, and return the result as a Value."
 
   { "post_event", gdbpy_post_event, METH_VARARGS,
     "Post an event into gdb's event loop." },
+  { "interrupt", gdbpy_interrupt, METH_NOARGS,
+    "Interrupt gdb's current operation." },
 
   { "target_charset", gdbpy_target_charset, METH_NOARGS,
     "target_charset () -> string.\n\

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 4/6] Rename a couple of DAP procs in the testsuite
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
                   ` (2 preceding siblings ...)
  2023-12-11 16:02 ` [PATCH v2 3/6] Introduce gdb.interrupt Tom Tromey
@ 2023-12-11 16:02 ` Tom Tromey
  2023-12-11 16:02 ` [PATCH v2 5/6] Catch KeyboardInterrupt in send_gdb_with_response Tom Tromey
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Kévin Le Gouguec

This renames a couple of DAP procs in the testsuite, to clarify that
they are now exported.  The cancellation test will need these.

Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
---
 gdb/testsuite/lib/dap-support.exp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/lib/dap-support.exp b/gdb/testsuite/lib/dap-support.exp
index 82d83d95b9f..b9ac314fee5 100644
--- a/gdb/testsuite/lib/dap-support.exp
+++ b/gdb/testsuite/lib/dap-support.exp
@@ -111,7 +111,7 @@ proc _dap_send_ton {obj} {
 # omitted.  The sequence number of the request is automatically added,
 # and this is also the return value.  OBJ is assumed to already be in
 # TON form.
-proc _dap_send_request {command {obj {}}} {
+proc dap_send_request {command {obj {}}} {
     # We can construct this directly as a TON object.
     set result $::dap_seq
     incr ::dap_seq
@@ -186,7 +186,7 @@ proc _dap_read_json {} {
 # response is seen, this leaves the global "last_ton" set to the TON
 # for the response.
 
-proc _dap_read_response {cmd num} {
+proc dap_read_response {cmd num} {
     set result {}
     while 1 {
 	set d [_dap_read_json]
@@ -204,11 +204,11 @@ proc _dap_read_response {cmd num} {
     }
 }
 
-# A wrapper for _dap_send_request and _dap_read_response.  This sends a
+# A wrapper for dap_send_request and dap_read_response.  This sends a
 # request to gdb and returns the response as a dict.
 proc dap_request_and_response {command {obj {}}} {
-    set seq [_dap_send_request $command $obj]
-    return [_dap_read_response $command $seq]
+    set seq [dap_send_request $command $obj]
+    return [dap_read_response $command $seq]
 }
 
 # Like dap_request_and_response, but also checks that the response

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 5/6] Catch KeyboardInterrupt in send_gdb_with_response
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
                   ` (3 preceding siblings ...)
  2023-12-11 16:02 ` [PATCH v2 4/6] Rename a couple of DAP procs in the testsuite Tom Tromey
@ 2023-12-11 16:02 ` Tom Tromey
  2023-12-11 17:53   ` Kévin Le Gouguec
  2023-12-11 16:02 ` [PATCH v2 6/6] Implement DAP cancellation Tom Tromey
  2023-12-11 18:53 ` [PATCH v2 0/6] " Tom Tromey
  6 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches

Cancellation will generally be seen by the DAP code as a
KeyboardInterrupt.  However, this derives from BaseException and not
Exception, so a small change is needed to send_gdb_with_response, to
forward the exception to the DAP server thread.
---
 gdb/python/lib/gdb/dap/startup.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/startup.py b/gdb/python/lib/gdb/dap/startup.py
index a16b51f7cf5..1d3b94762a6 100644
--- a/gdb/python/lib/gdb/dap/startup.py
+++ b/gdb/python/lib/gdb/dap/startup.py
@@ -172,11 +172,11 @@ def send_gdb_with_response(fn):
         try:
             val = fn()
             result_q.put(val)
-        except Exception as e:
+        except (Exception, KeyboardInterrupt) as e:
             result_q.put(e)
 
     send_gdb(message)
     val = result_q.get()
-    if isinstance(val, Exception):
+    if isinstance(val, (Exception, KeyboardInterrupt)):
         raise val
     return val

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 6/6] Implement DAP cancellation
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
                   ` (4 preceding siblings ...)
  2023-12-11 16:02 ` [PATCH v2 5/6] Catch KeyboardInterrupt in send_gdb_with_response Tom Tromey
@ 2023-12-11 16:02 ` Tom Tromey
  2023-12-11 18:53 ` [PATCH v2 0/6] " Tom Tromey
  6 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 16:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Eli Zaretskii, Kévin Le Gouguec

This implements DAP cancellation.  A new object is introduced that
handles the details of cancellation.  While cancellation is inherently
racy, this code attempts to make it so that gdb doesn't inadvertently
cancel the wrong request.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30472
Approved-By: Eli Zaretskii <eliz@gnu.org>
Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
---
 gdb/NEWS                         |  2 +
 gdb/doc/gdb.texinfo              | 16 +++++++
 gdb/python/lib/gdb/dap/server.py | 91 ++++++++++++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.dap/pause.exp  | 71 +++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 900ac47ada9..3e804fb1e53 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -82,6 +82,8 @@ show remote thread-options-packet
 
   ** GDB now emits the "process" event.
 
+  ** GDB now supports the "cancel" request.
+
 * New remote packets
 
 New stop reason: clone
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 58685a77fd4..6e4adf512ee 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -39595,6 +39595,22 @@ to return the bytes of each instruction in an implementation-defined
 format.  @value{GDBN} implements this by sending a string with the
 bytes encoded in hex, like @code{"55a2b900"}.
 
+When the @code{repl} context is used for the @code{evaluate} request,
+@value{GDBN} evaluates the provided expression as a CLI command.
+
+Evaluation in general can cause the inferior to continue execution.
+For example, evaluating the @code{continue} command could do this, as
+could evaluating an expression that involves an inferior function
+call.
+
+@code{repl} evaluation can also cause @value{GDBN} to appear to stop
+responding to requests, for example if a CLI script does a lengthy
+computation.
+
+Evaluations like this can be interrupted using the DAP @code{cancel}
+request.  (In fact, @code{cancel} should work for any request, but it
+is unlikely to be useful for most of them.)
+
 @node JIT Interface
 @chapter JIT Compilation Interface
 @cindex just-in-time compilation
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index 53a0ca7f448..44dffb1b809 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -14,8 +14,11 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import functools
+import gdb
+import heapq
 import inspect
 import json
+import threading
 
 from .io import start_json_writer, read_json
 from .startup import (
@@ -48,6 +51,52 @@ class NotStoppedException(Exception):
     pass
 
 
+# This is used to handle cancellation requests.  It tracks all the
+# needed state, so that we can cancel both requests that are in flight
+# as well as queued requests.
+class CancellationHandler:
+    def __init__(self):
+        # Methods on this class acquire this lock before proceeding.
+        self.lock = threading.Lock()
+        # The request currently being handled, or None.
+        self.in_flight = None
+        self.reqs = []
+
+    def starting(self, req):
+        """Call at the start of the given request.
+
+        Throws the appropriate exception if the request should be
+        immediately cancelled."""
+        with self.lock:
+            self.in_flight = req
+            while len(self.reqs) > 0 and self.reqs[0] <= req:
+                if heapq.heappop(self.reqs) == req:
+                    raise KeyboardInterrupt()
+
+    def done(self, req):
+        """Indicate that the request is done."""
+        with self.lock:
+            self.in_flight = None
+
+    def cancel(self, req):
+        """Call to cancel a request.
+
+        If the request has already finished, this is ignored.
+        If the request is in flight, it is interrupted.
+        If the request has not yet been seen, the cancellation is queued."""
+        with self.lock:
+            if req == self.in_flight:
+                gdb.interrupt()
+            else:
+                # We don't actually ignore the request here, but in
+                # the 'starting' method.  This way we don't have to
+                # track as much state.  Also, this implementation has
+                # the weird property that a request can be cancelled
+                # before it is even sent.  It didn't seem worthwhile
+                # to try to check for this.
+                heapq.heappush(self.reqs, req)
+
+
 class Server:
     """The DAP server class."""
 
@@ -64,6 +113,7 @@ class Server:
         # requests is kept.
         self.read_queue = DAPQueue()
         self.done = False
+        self.canceller = CancellationHandler()
         global _server
         _server = self
 
@@ -71,13 +121,14 @@ class Server:
     # PARAMS is just a dictionary from the JSON.
     @in_dap_thread
     def _handle_command(self, params):
-        # We don't handle 'cancel' for now.
+        req = params["seq"]
         result = {
-            "request_seq": params["seq"],
+            "request_seq": req,
             "type": "response",
             "command": params["command"],
         }
         try:
+            self.canceller.starting(req)
             if "arguments" in params:
                 args = params["arguments"]
             else:
@@ -90,10 +141,15 @@ class Server:
         except NotStoppedException:
             result["success"] = False
             result["message"] = "notStopped"
+        except KeyboardInterrupt:
+            # This can only happen when a request has been canceled.
+            result["success"] = False
+            result["message"] = "cancelled"
         except BaseException as e:
             log_stack()
             result["success"] = False
             result["message"] = str(e)
+        self.canceller.done(req)
         return result
 
     # Read inferior output and sends OutputEvents to the client.  It
@@ -115,11 +171,25 @@ class Server:
         self.write_queue.put(obj)
 
     # This is run in a separate thread and simply reads requests from
-    # the client and puts them into a queue.
+    # the client and puts them into a queue.  A separate thread is
+    # used so that 'cancel' requests can be handled -- the DAP thread
+    # will normally block, waiting for each request to complete.
     def _reader_thread(self):
         while True:
             cmd = read_json(self.in_stream)
             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
+            # issued by ordinary request processing.
+            if (
+                "command" in cmd
+                and cmd["command"] == "cancel"
+                and "arguments" in cmd
+                # gdb does not implement progress, so there's no need
+                # to check for progressId.
+                and "requestId" in cmd["arguments"]
+            ):
+                self.canceller.cancel(cmd["arguments"]["requestId"])
             self.read_queue.put(cmd)
 
     @in_dap_thread
@@ -316,3 +386,18 @@ def disconnect(*, terminateDebuggee: bool = False, **args):
     if terminateDebuggee:
         send_gdb_with_response("kill")
     _server.shutdown()
+
+
+@request("cancel", on_dap_thread=True, expect_stopped=False)
+@capability("supportsCancelRequest")
+def cancel(**args):
+    # If a 'cancel' request can actually be satisfied, it will be
+    # handled specially in the reader thread.  However, in order to
+    # construct a proper response, the request is also added to the
+    # command queue and so ends up here.  Additionally, the spec says:
+    #    The cancel request may return an error if it could not cancel
+    #    an operation but a client should refrain from presenting this
+    #    error to end users.
+    # ... which gdb takes to mean that it is fine for all cancel
+    # requests to report success.
+    return None
diff --git a/gdb/testsuite/gdb.dap/pause.exp b/gdb/testsuite/gdb.dap/pause.exp
index 1b65dcac836..0f2fcbc6b2d 100644
--- a/gdb/testsuite/gdb.dap/pause.exp
+++ b/gdb/testsuite/gdb.dap/pause.exp
@@ -75,4 +75,75 @@ foreach event [lindex $result 1] {
 }
 gdb_assert {$seen == "pass"} "continue event from inferior call"
 
+#
+# Test that a repl evaluation that causes a continue can be canceled.
+#
+
+set cont_id [dap_send_request evaluate \
+		 {o expression [s continue] context [s repl]}]
+dap_wait_for_event_and_check "continued" continued
+
+set cancel_id [dap_send_request cancel \
+		   [format {o requestId [i %d]} $cont_id]]
+
+# The stop event will come before any responses to the requests.
+dap_wait_for_event_and_check "stopped by cancel" stopped
+
+# Now we can wait for the 'continue' request to complete, and then the
+# 'cancel' request.
+dap_read_response evaluate $cont_id
+dap_read_response cancel $cancel_id
+
+#
+# Test that a repl evaluation of a long-running gdb command (that does
+# not continue the inferior) can be canceled.
+#
+
+proc write_file {suffix contents} {
+    global testfile
+
+    set gdbfile [standard_output_file ${testfile}.$suffix]
+    set ofd [open $gdbfile w]
+    puts $ofd $contents
+    close $ofd
+    return $gdbfile
+}
+
+set gdbfile [write_file gdb "set \$x = 0\nwhile 1\nset \$x = \$x + 1\nend"]
+set cont_id [dap_send_request evaluate \
+		 [format {o expression [s "source %s"] context [s repl]} \
+		      $gdbfile]]
+
+# Wait a little to try to ensure the command is running.
+sleep 0.2
+set cancel_id [dap_send_request cancel \
+		   [format {o requestId [i %d]} $cont_id]]
+
+set info [lindex [dap_read_response evaluate $cont_id] 0]
+gdb_assert {[dict get $info success] == "false"} "gdb command failed"
+gdb_assert {[dict get $info message] == "cancelled"} "gdb command canceled"
+
+dap_read_response cancel $cancel_id
+
+#
+# Test that a repl evaluation of a long-running Python command (that
+# does not continue the inferior) can be canceled.
+#
+
+write_file py "while True:\n  pass"
+set cont_id [dap_send_request evaluate \
+		 [format {o expression [s "source %s"] context [s repl]} \
+		      $gdbfile]]
+
+# Wait a little to try to ensure the command is running.
+sleep 0.2
+set cancel_id [dap_send_request cancel \
+		   [format {o requestId [i %d]} $cont_id]]
+
+set info [lindex [dap_read_response evaluate $cont_id] 0]
+gdb_assert {[dict get $info success] == "false"} "python command failed"
+gdb_assert {[dict get $info message] == "cancelled"} "python command canceled"
+
+dap_read_response cancel $cancel_id
+
 dap_shutdown

-- 
2.43.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/6] Introduce gdb.interrupt
  2023-12-11 16:02 ` [PATCH v2 3/6] Introduce gdb.interrupt Tom Tromey
@ 2023-12-11 16:30   ` Eli Zaretskii
  0 siblings, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2023-12-11 16:30 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, legouguec

> From: Tom Tromey <tromey@adacore.com>
> Date: Mon, 11 Dec 2023 09:02:20 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>, 
>  Kévin Le Gouguec <legouguec@adacore.com>
> 
> DAP cancellation needs a way to interrupt whatever is happening on
> gdb's main thread -- whether that is the inferior, a gdb CLI command,
> or Python code.
> 
> This patch adds a new gdb.interrupt() function for this purpose.  It
> simply sets the quit flag and lets gdb do the rest.
> 
> No tests in this patch -- instead this is tested via the DAP
> cancellation tests.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>
> ---
>  gdb/NEWS            |  3 +++
>  gdb/doc/python.texi | 11 +++++++++++
>  gdb/python/python.c | 19 +++++++++++++++++++
>  3 files changed, 33 insertions(+)

Thanks, the documentation parts are approved.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 5/6] Catch KeyboardInterrupt in send_gdb_with_response
  2023-12-11 16:02 ` [PATCH v2 5/6] Catch KeyboardInterrupt in send_gdb_with_response Tom Tromey
@ 2023-12-11 17:53   ` Kévin Le Gouguec
  0 siblings, 0 replies; 10+ messages in thread
From: Kévin Le Gouguec @ 2023-12-11 17:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Tom Tromey <tromey@adacore.com> writes:

> Cancellation will generally be seen by the DAP code as a
> KeyboardInterrupt.  However, this derives from BaseException and not
> Exception, so a small change is needed to send_gdb_with_response, to
> forward the exception to the DAP server thread.
> ---
>  gdb/python/lib/gdb/dap/startup.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/python/lib/gdb/dap/startup.py b/gdb/python/lib/gdb/dap/startup.py
> index a16b51f7cf5..1d3b94762a6 100644
> --- a/gdb/python/lib/gdb/dap/startup.py
> +++ b/gdb/python/lib/gdb/dap/startup.py
> @@ -172,11 +172,11 @@ def send_gdb_with_response(fn):
>          try:
>              val = fn()
>              result_q.put(val)
> -        except Exception as e:
> +        except (Exception, KeyboardInterrupt) as e:
>              result_q.put(e)
>  
>      send_gdb(message)
>      val = result_q.get()
> -    if isinstance(val, Exception):
> +    if isinstance(val, (Exception, KeyboardInterrupt)):
>          raise val
>      return val

ACK!

Reviewed-By: Kévin Le Gouguec <legouguec@adacore.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/6] Implement DAP cancellation
  2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
                   ` (5 preceding siblings ...)
  2023-12-11 16:02 ` [PATCH v2 6/6] Implement DAP cancellation Tom Tromey
@ 2023-12-11 18:53 ` Tom Tromey
  6 siblings, 0 replies; 10+ messages in thread
From: Tom Tromey @ 2023-12-11 18:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Kévin Le Gouguec, Eli Zaretskii

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> DAP specifies a way to cancel requests.  Previously, I didn't
Tom> implement this, because it didn't seem necessary.  However, I realized
Tom> later that a 'repl' evaluation can cause the inferior to continue (or
Tom> gdb to invoke a long-running CLI or Python command), and a request
Tom> like this will not finish -- cancellation is necessary.

Tom> This series implements this feature.  I've split it into bite-sized
Tom> pieces.

Tom> Tested on x86-64 Fedora 38.

I'm going to check this in now.

Tom

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-12-11 18:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-11 16:02 [PATCH v2 0/6] Implement DAP cancellation Tom Tromey
2023-12-11 16:02 ` [PATCH v2 1/6] Clean up handling of DAP not-stopped response Tom Tromey
2023-12-11 16:02 ` [PATCH v2 2/6] Move DAP JSON reader to its own thread Tom Tromey
2023-12-11 16:02 ` [PATCH v2 3/6] Introduce gdb.interrupt Tom Tromey
2023-12-11 16:30   ` Eli Zaretskii
2023-12-11 16:02 ` [PATCH v2 4/6] Rename a couple of DAP procs in the testsuite Tom Tromey
2023-12-11 16:02 ` [PATCH v2 5/6] Catch KeyboardInterrupt in send_gdb_with_response Tom Tromey
2023-12-11 17:53   ` Kévin Le Gouguec
2023-12-11 16:02 ` [PATCH v2 6/6] Implement DAP cancellation Tom Tromey
2023-12-11 18:53 ` [PATCH v2 0/6] " Tom Tromey

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).