[PATCH v2 0/3] Implement the DAP notStopped response

[PATCH v2 0/3] Implement the DAP notStopped response

[Tom Tromey]

This series implements the DAP 'notStopped' response.
Tested on x86-64 Fedora 38.

This probably conflicts with other DAP patches I've sent, but of
course I'll rebase & fix up as needed if/when those land.

---
Changes in v2:
- Changed a few names as suggested by Kévin
- Removed a FIXME in favor of an explanation
- Link to v1: https://inbox.sourceware.org/gdb-patches/20231107-dap-not-stopped-v1-0-3d91c935255d@adacore.com

---
Tom Tromey (3):
      Automatically run (most) DAP requests in gdb thread
      Remove ExecutionInvoker
      Implement the notStopped DAP response

 gdb/python/lib/gdb/dap/breakpoint.py  |  13 ++---
 gdb/python/lib/gdb/dap/bt.py          |  23 +++-----
 gdb/python/lib/gdb/dap/disassemble.py |   6 +-
 gdb/python/lib/gdb/dap/evaluate.py    |  23 +++-----
 gdb/python/lib/gdb/dap/events.py      |  44 +++++++-------
 gdb/python/lib/gdb/dap/launch.py      |  23 +++-----
 gdb/python/lib/gdb/dap/locations.py   |   4 +-
 gdb/python/lib/gdb/dap/memory.py      |  21 ++-----
 gdb/python/lib/gdb/dap/modules.py     |   4 +-
 gdb/python/lib/gdb/dap/next.py        |  29 ++++++----
 gdb/python/lib/gdb/dap/pause.py       |   7 +--
 gdb/python/lib/gdb/dap/scopes.py      |   5 +-
 gdb/python/lib/gdb/dap/server.py      | 104 ++++++++++++++++++++++++++++------
 gdb/python/lib/gdb/dap/sources.py     |  32 ++++-------
 gdb/python/lib/gdb/dap/threads.py     |  12 +---
 gdb/testsuite/gdb.dap/pause.exp       |   7 +++
 16 files changed, 188 insertions(+), 169 deletions(-)
---
base-commit: 3cb843c9df28a2a4cbb6d6e6c96fda2a6ca26295
change-id: 20231107-dap-not-stopped-361e6df17333

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

[PATCH v2 1/3] Automatically run (most) DAP requests in gdb thread

[Tom Tromey]

Nearly every DAP request implementation forwards its work to the gdb
thread, using send_gdb_with_response.  This patch refactors the
'request' decorator to make this automatic, and to provide some
parameters so that the unusual requests can express their needs as
well.

In a few spots this simplifies the code by removing an unnecessary
helper function.  This could be done in more places as well if we
wanted.

The main motivation for this patch is that I thought it would be
helpful for cancellation.  I am still working on that, but meanwhile
the parameterization of 'request' makes it easy to handle the
'notStopped' response as well.
---
 gdb/python/lib/gdb/dap/breakpoint.py  | 13 +++----
 gdb/python/lib/gdb/dap/bt.py          | 23 ++++--------
 gdb/python/lib/gdb/dap/disassemble.py |  6 +--
 gdb/python/lib/gdb/dap/evaluate.py    | 23 ++++--------
 gdb/python/lib/gdb/dap/launch.py      | 21 ++++-------
 gdb/python/lib/gdb/dap/locations.py   |  4 +-
 gdb/python/lib/gdb/dap/memory.py      | 21 ++---------
 gdb/python/lib/gdb/dap/modules.py     |  4 +-
 gdb/python/lib/gdb/dap/next.py        | 25 ++++++++-----
 gdb/python/lib/gdb/dap/pause.py       |  5 +--
 gdb/python/lib/gdb/dap/scopes.py      |  5 +--
 gdb/python/lib/gdb/dap/server.py      | 69 +++++++++++++++++++++++++++--------
 gdb/python/lib/gdb/dap/sources.py     | 32 +++++-----------
 gdb/python/lib/gdb/dap/threads.py     | 12 +-----
 14 files changed, 121 insertions(+), 142 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index ae3a5027f35..33aa18e65bc 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -24,7 +24,7 @@ from typing import Optional, Sequence
 
 from .server import request, capability, send_event
 from .sources import make_source
-from .startup import send_gdb_with_response, in_gdb_thread, log_stack
+from .startup import in_gdb_thread, log_stack
 from .typecheck import type_check
 
 
@@ -287,7 +287,7 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
         # Be sure to include the path in the key, so that we only
         # clear out breakpoints coming from this same source.
         key = "source:" + source["path"]
-        result = send_gdb_with_response(lambda: _set_breakpoints(key, specs))
+        result = _set_breakpoints(key, specs)
     return {
         "breakpoints": result,
     }
@@ -315,9 +315,8 @@ def _rewrite_fn_breakpoint(
 @capability("supportsFunctionBreakpoints")
 def set_fn_breakpoint(*, breakpoints: Sequence, **args):
     specs = [_rewrite_fn_breakpoint(**bp) for bp in breakpoints]
-    result = send_gdb_with_response(lambda: _set_breakpoints("function", specs))
     return {
-        "breakpoints": result,
+        "breakpoints": _set_breakpoints("function", specs),
     }
 
 
@@ -351,9 +350,8 @@ def set_insn_breakpoints(
     *, breakpoints: Sequence, offset: Optional[int] = None, **args
 ):
     specs = [_rewrite_insn_breakpoint(**bp) for bp in breakpoints]
-    result = send_gdb_with_response(lambda: _set_breakpoints("instruction", specs))
     return {
-        "breakpoints": result,
+        "breakpoints": _set_breakpoints("instruction", specs),
     }
 
 
@@ -432,7 +430,6 @@ def set_exception_breakpoints(
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
     options = [_rewrite_exception_breakpoint(**bp) for bp in options]
-    result = send_gdb_with_response(lambda: _set_exception_catchpoints(options))
     return {
-        "breakpoints": result,
+        "breakpoints": _set_exception_catchpoints(options),
     }
diff --git a/gdb/python/lib/gdb/dap/bt.py b/gdb/python/lib/gdb/dap/bt.py
index 982d501edf5..df388c11469 100644
--- a/gdb/python/lib/gdb/dap/bt.py
+++ b/gdb/python/lib/gdb/dap/bt.py
@@ -21,16 +21,17 @@ from .frames import frame_id
 from .modules import module_id
 from .server import request, capability
 from .sources import make_source
-from .startup import send_gdb_with_response, in_gdb_thread
 from .state import set_thread
 from .varref import apply_format
 
 
-# Helper function to compute a stack trace.
-@in_gdb_thread
-def _backtrace(thread_id, levels, startFrame, value_format):
-    with apply_format(value_format):
-        set_thread(thread_id)
+@request("stackTrace")
+@capability("supportsDelayedStackTraceLoading")
+def stacktrace(
+    *, levels: int = 0, startFrame: int = 0, threadId: int, format=None, **extra
+):
+    with apply_format(format):
+        set_thread(threadId)
         frames = []
         if levels == 0:
             # Zero means all remaining frames.
@@ -70,13 +71,3 @@ def _backtrace(thread_id, levels, startFrame, value_format):
         return {
             "stackFrames": frames,
         }
-
-
-@request("stackTrace")
-@capability("supportsDelayedStackTraceLoading")
-def stacktrace(
-    *, levels: int = 0, startFrame: int = 0, threadId: int, format=None, **extra
-):
-    return send_gdb_with_response(
-        lambda: _backtrace(threadId, levels, startFrame, format)
-    )
diff --git a/gdb/python/lib/gdb/dap/disassemble.py b/gdb/python/lib/gdb/dap/disassemble.py
index dda2f43b5a3..069549eb7f8 100644
--- a/gdb/python/lib/gdb/dap/disassemble.py
+++ b/gdb/python/lib/gdb/dap/disassemble.py
@@ -16,7 +16,7 @@
 import gdb
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 
 
 @in_gdb_thread
@@ -54,6 +54,4 @@ def disassemble(
     **extra
 ):
     pc = int(memoryReference, 0) + offset
-    return send_gdb_with_response(
-        lambda: _disassemble(pc, instructionOffset, instructionCount)
-    )
+    return _disassemble(pc, instructionOffset, instructionCount)
diff --git a/gdb/python/lib/gdb/dap/evaluate.py b/gdb/python/lib/gdb/dap/evaluate.py
index ea5a1e61a08..67e103e2ca7 100644
--- a/gdb/python/lib/gdb/dap/evaluate.py
+++ b/gdb/python/lib/gdb/dap/evaluate.py
@@ -20,7 +20,7 @@ from typing import Optional
 
 from .frames import select_frame
 from .server import capability, request, client_bool_capability
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 from .varref import find_variable, VariableReference, apply_format
 
 
@@ -96,14 +96,12 @@ def eval_request(
 ):
     if context in ("watch", "variables"):
         # These seem to be expression-like.
-        return send_gdb_with_response(lambda: _evaluate(expression, frameId, format))
+        return _evaluate(expression, frameId, format)
     elif context == "hover":
-        return send_gdb_with_response(
-            lambda: _eval_for_hover(expression, frameId, format)
-        )
+        return _eval_for_hover(expression, frameId, format)
     elif context == "repl":
         # Ignore the format for repl evaluation.
-        return send_gdb_with_response(lambda: _repl(expression, frameId))
+        return _repl(expression, frameId)
     else:
         raise Exception('unknown evaluate context "' + context + '"')
 
@@ -127,10 +125,7 @@ def variables(
     if not client_bool_capability("supportsVariablePaging"):
         start = 0
         count = 0
-    result = send_gdb_with_response(
-        lambda: _variables(variablesReference, start, count, format)
-    )
-    return {"variables": result}
+    return {"variables": _variables(variablesReference, start, count, format)}
 
 
 @capability("supportsSetExpression")
@@ -138,9 +133,7 @@ def variables(
 def set_expression(
     *, expression: str, value: str, frameId: Optional[int] = None, format=None, **args
 ):
-    return send_gdb_with_response(
-        lambda: _set_expression(expression, value, frameId, format)
-    )
+    return _set_expression(expression, value, frameId, format)
 
 
 # Helper function to perform an assignment.
@@ -159,6 +152,4 @@ def _set_variable(ref, name, value, value_format):
 def set_variable(
     *, variablesReference: int, name: str, value: str, format=None, **args
 ):
-    return send_gdb_with_response(
-        lambda: _set_variable(variablesReference, name, value, format)
-    )
+    return _set_variable(variablesReference, name, value, format)
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index d13037fa476..e81d2849a8e 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -20,11 +20,11 @@ from typing import Mapping, Optional, Sequence
 
 from .events import ExecutionInvoker
 from .server import request, capability
-from .startup import send_gdb, send_gdb_with_response, in_gdb_thread, exec_and_log
+from .startup import in_gdb_thread, exec_and_log
 
 
-# The program being launched, or None.  This should only be access
-# from the DAP thread.
+# The program being launched, or None.  This should only be accessed
+# from the gdb thread.
 _program = None
 
 
@@ -49,7 +49,7 @@ def _launch_setup(program, cwd, args, env, stopAtBeginningOfMainSubprogram):
 # Any parameters here are necessarily extensions -- DAP requires this
 # from implementations.  Any additions or changes here should be
 # documented in the gdb manual.
-@request("launch")
+@request("launch", response=False)
 def launch(
     *,
     program: Optional[str] = None,
@@ -61,9 +61,7 @@ def launch(
 ):
     global _program
     _program = program
-    send_gdb(
-        lambda: _launch_setup(program, cwd, args, env, stopAtBeginningOfMainSubprogram)
-    )
+    _launch_setup(program, cwd, args, env, stopAtBeginningOfMainSubprogram)
 
 
 @request("attach")
@@ -77,17 +75,14 @@ def attach(*, pid: Optional[int] = None, target: Optional[str] = None, **args):
         cmd = "target remote " + target
     else:
         raise Exception("attach requires either 'pid' or 'target'")
-    # Use send_gdb_with_response to ensure we get an error if the
-    # attach fails.
-    send_gdb_with_response(cmd)
-    return None
+    exec_and_log(cmd)
 
 
 @capability("supportsConfigurationDoneRequest")
-@request("configurationDone")
+@request("configurationDone", response=False)
 def config_done(**args):
     global _program
     if _program is not None:
         # Suppress the continue event, but don't set any particular
         # expected stop.
-        send_gdb(ExecutionInvoker("run", None))
+        ExecutionInvoker("run", None)()
diff --git a/gdb/python/lib/gdb/dap/locations.py b/gdb/python/lib/gdb/dap/locations.py
index a299e8da959..032174df9c8 100644
--- a/gdb/python/lib/gdb/dap/locations.py
+++ b/gdb/python/lib/gdb/dap/locations.py
@@ -20,7 +20,7 @@ from typing import Optional
 
 from .server import capability, request
 from .sources import decode_source
-from .startup import in_gdb_thread, send_gdb_with_response
+from .startup import in_gdb_thread
 
 
 @in_gdb_thread
@@ -46,4 +46,4 @@ def _find_lines(source, start_line, end_line):
 def breakpoint_locations(*, source, line: int, endLine: Optional[int] = None, **extra):
     if endLine is None:
         endLine = line
-    return send_gdb_with_response(lambda: _find_lines(source, line, endLine))
+    return _find_lines(source, line, endLine)
diff --git a/gdb/python/lib/gdb/dap/memory.py b/gdb/python/lib/gdb/dap/memory.py
index 85948bda9f4..6b94f413045 100644
--- a/gdb/python/lib/gdb/dap/memory.py
+++ b/gdb/python/lib/gdb/dap/memory.py
@@ -17,35 +17,22 @@ import base64
 import gdb
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
-
-
-@in_gdb_thread
-def _read_memory(addr, count):
-    buf = gdb.selected_inferior().read_memory(addr, count)
-    return base64.b64encode(buf).decode("ASCII")
 
 
 @request("readMemory")
 @capability("supportsReadMemoryRequest")
 def read_memory(*, memoryReference: str, offset: int = 0, count: int, **extra):
     addr = int(memoryReference, 0) + offset
-    buf = send_gdb_with_response(lambda: _read_memory(addr, count))
+    buf = gdb.selected_inferior().read_memory(addr, count)
     return {
         "address": hex(addr),
-        "data": buf,
+        "data": base64.b64encode(buf).decode("ASCII"),
     }
 
 
-@in_gdb_thread
-def _write_memory(addr, contents):
-    buf = base64.b64decode(contents)
-    gdb.selected_inferior().write_memory(addr, buf)
-
-
 @request("writeMemory")
 @capability("supportsWriteMemoryRequest")
 def write_memory(*, memoryReference: str, offset: int = 0, data: str, **extra):
     addr = int(memoryReference, 0) + offset
-    send_gdb_with_response(lambda: _write_memory(addr, data))
-    return {}
+    buf = base64.b64decode(data)
+    gdb.selected_inferior().write_memory(addr, buf)
diff --git a/gdb/python/lib/gdb/dap/modules.py b/gdb/python/lib/gdb/dap/modules.py
index 1aec1cba0ac..87a4f6be669 100644
--- a/gdb/python/lib/gdb/dap/modules.py
+++ b/gdb/python/lib/gdb/dap/modules.py
@@ -16,7 +16,7 @@
 import gdb
 
 from .server import capability, request
-from .startup import in_gdb_thread, send_gdb_with_response
+from .startup import in_gdb_thread
 
 
 @in_gdb_thread
@@ -63,4 +63,4 @@ def _modules(start, count):
 @capability("supportsModulesRequest")
 @request("modules")
 def modules(*, startModule: int = 0, moduleCount: int = 0, **args):
-    return send_gdb_with_response(lambda: _modules(startModule, moduleCount))
+    return _modules(startModule, moduleCount)
diff --git a/gdb/python/lib/gdb/dap/next.py b/gdb/python/lib/gdb/dap/next.py
index e5bb8d64da0..431020e32e1 100644
--- a/gdb/python/lib/gdb/dap/next.py
+++ b/gdb/python/lib/gdb/dap/next.py
@@ -49,37 +49,42 @@ def _handle_thread_step(thread_id, single_thread, select=False):
     return result
 
 
-@request("next")
+@request("next", response=False)
 def next(
     *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
 ):
-    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
+    _handle_thread_step(threadId, singleThread)
     cmd = "next"
     if granularity == "instruction":
         cmd += "i"
-    send_gdb(ExecutionInvoker(cmd, StopKinds.STEP))
+    ExecutionInvoker(cmd, StopKinds.STEP)()
 
 
 @capability("supportsSteppingGranularity")
 @capability("supportsSingleThreadExecutionRequests")
-@request("stepIn")
+@request("stepIn", response=False)
 def step_in(
     *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
 ):
-    send_gdb(lambda: _handle_thread_step(threadId, singleThread))
+    _handle_thread_step(threadId, singleThread)
     cmd = "step"
     if granularity == "instruction":
         cmd += "i"
-    send_gdb(ExecutionInvoker(cmd, StopKinds.STEP))
+    ExecutionInvoker(cmd, StopKinds.STEP)()
 
 
-@request("stepOut")
+@request("stepOut", response=False)
 def step_out(*, threadId: int, singleThread: bool = False, **args):
-    send_gdb(lambda: _handle_thread_step(threadId, singleThread, True))
-    send_gdb(ExecutionInvoker("finish", StopKinds.STEP))
+    _handle_thread_step(threadId, singleThread, True)
+    ExecutionInvoker("finish", StopKinds.STEP)()
 
 
-@request("continue")
+# This is a server-side request because it is funny: it wants to
+# 'continue' but also return a result, which precludes using
+# response=False.  Using 'continue &' would mostly work ok, but this
+# yields races when a stop occurs before the response is sent back to
+# the client.
+@request("continue", on_dap_thread=True)
 def continue_request(*, threadId: int, singleThread: bool = False, **args):
     locked = send_gdb_with_response(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("continue", None))
diff --git a/gdb/python/lib/gdb/dap/pause.py b/gdb/python/lib/gdb/dap/pause.py
index 1e59d630523..d96172c0757 100644
--- a/gdb/python/lib/gdb/dap/pause.py
+++ b/gdb/python/lib/gdb/dap/pause.py
@@ -15,9 +15,8 @@
 
 from .events import StopKinds, ExecutionInvoker
 from .server import request
-from .startup import send_gdb
 
 
-@request("pause")
+@request("pause", response=False)
 def pause(**args):
-    send_gdb(ExecutionInvoker("interrupt -a", StopKinds.PAUSE))
+    ExecutionInvoker("interrupt -a", StopKinds.PAUSE)()
diff --git a/gdb/python/lib/gdb/dap/scopes.py b/gdb/python/lib/gdb/dap/scopes.py
index 87f2ed7547f..d0c30c6b501 100644
--- a/gdb/python/lib/gdb/dap/scopes.py
+++ b/gdb/python/lib/gdb/dap/scopes.py
@@ -16,7 +16,7 @@
 import gdb
 
 from .frames import frame_for_id
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 from .server import request
 from .varref import BaseReference
 
@@ -120,5 +120,4 @@ def _get_scope(id):
 
 @request("scopes")
 def scopes(*, frameId: int, **extra):
-    scopes = send_gdb_with_response(lambda: _get_scope(frameId))
-    return {"scopes": scopes}
+    return {"scopes": _get_scope(frameId)}
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index 62bf240c1e9..4430d2aba5f 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -20,11 +20,14 @@ import sys
 
 from .io import start_json_writer, read_json
 from .startup import (
+    exec_and_log,
     in_dap_thread,
+    in_gdb_thread,
+    send_gdb,
+    send_gdb_with_response,
     start_thread,
     log,
     log_stack,
-    send_gdb_with_response,
 )
 from .typecheck import type_check
 
@@ -160,12 +163,27 @@ def send_event(event, body=None):
     _server.send_event(event, body)
 
 
-def request(name):
-    """A decorator that indicates that the wrapper function implements
-    the DAP request NAME."""
+def request(name: str, *, response: bool = True, on_dap_thread: bool = False):
+    """A decorator for DAP requests.
+
+    This registers the function as the implementation of the DAP
+    request NAME.  By default, the function is invoked in the gdb
+    thread, and its result is returned as the 'body' of the DAP
+    response.
+
+    Some keyword arguments are provided as well:
+
+    If RESPONSE is False, the result of the function will not be
+    waited for and no 'body' will be in the response.
+
+    If ON_DAP_THREAD is True, the function will be invoked in the DAP
+    thread.  When ON_DAP_THREAD is True, RESPONSE may not be False.
+    """
+
+    # Validate the parameters.
+    assert not on_dap_thread or response
 
     def wrap(func):
-        global _commands
         code = func.__code__
         # We don't permit requests to have positional arguments.
         try:
@@ -176,11 +194,32 @@ def request(name):
         assert code.co_argcount == 0
         # A request must have a **args parameter.
         assert code.co_flags & inspect.CO_VARKEYWORDS
-        # All requests must run in the DAP thread.
-        # Also type-check the calls.
-        func = in_dap_thread(type_check(func))
-        _commands[name] = func
-        return func
+
+        # Type-check the calls.
+        func = type_check(func)
+
+        # Verify that the function is run on the correct thread.
+        if on_dap_thread:
+            cmd = in_dap_thread(func)
+        else:
+            func = in_gdb_thread(func)
+
+            if response:
+
+                def sync_call(**args):
+                    return send_gdb_with_response(lambda: func(**args))
+
+                cmd = sync_call
+            else:
+
+                def non_sync_call(**args):
+                    return send_gdb(lambda: func(**args))
+
+                cmd = non_sync_call
+
+        global _commands
+        _commands[name] = cmd
+        return cmd
 
     return wrap
 
@@ -208,7 +247,7 @@ def client_bool_capability(name):
     return False
 
 
-@request("initialize")
+@request("initialize", on_dap_thread=True)
 def initialize(**args):
     global _server, _capabilities
     _server.config = args
@@ -219,14 +258,12 @@ def initialize(**args):
 @request("terminate")
 @capability("supportsTerminateRequest")
 def terminate(**args):
-    # We can ignore the result here, because we only really need to
-    # synchronize.
-    send_gdb_with_response("kill")
+    exec_and_log("kill")
 
 
-@request("disconnect")
+@request("disconnect", on_dap_thread=True)
 @capability("supportTerminateDebuggee")
 def disconnect(*, terminateDebuggee: bool = False, **args):
     if terminateDebuggee:
-        terminate()
+        send_gdb_with_response("kill")
     _server.shutdown()
diff --git a/gdb/python/lib/gdb/dap/sources.py b/gdb/python/lib/gdb/dap/sources.py
index 00a70701d26..821205cedd1 100644
--- a/gdb/python/lib/gdb/dap/sources.py
+++ b/gdb/python/lib/gdb/dap/sources.py
@@ -18,7 +18,7 @@ import os
 import gdb
 
 from .server import request, capability
-from .startup import send_gdb_with_response, in_gdb_thread
+from .startup import in_gdb_thread
 
 
 # The next available source reference ID.  Must be greater than 0.
@@ -76,8 +76,9 @@ def decode_source(source):
     return _id_map[ref]["path"]
 
 
-@in_gdb_thread
-def _sources():
+@request("loadedSources")
+@capability("supportsLoadedSourcesRequest")
+def loaded_sources(**extra):
     result = []
     for elt in gdb.execute_mi("-file-list-exec-source-files")["files"]:
         result.append(make_source(elt["fullname"], elt["file"]))
@@ -86,24 +87,6 @@ def _sources():
     }
 
 
-@request("loadedSources")
-@capability("supportsLoadedSourcesRequest")
-def loaded_sources(**extra):
-    return send_gdb_with_response(_sources)
-
-
-# This helper is needed because we must only access the globals here
-# from the gdb thread.
-@in_gdb_thread
-def _get_source(source):
-    filename = decode_source(source)
-    with open(filename) as f:
-        content = f.read()
-    return {
-        "content": content,
-    }
-
-
 @request("source")
 def source(*, source=None, sourceReference: int, **extra):
     # The 'sourceReference' parameter is required by the spec, but is
@@ -111,4 +94,9 @@ def source(*, source=None, sourceReference: int, **extra):
     # 'source' is preferred.
     if source is None:
         source = {"sourceReference": sourceReference}
-    return send_gdb_with_response(lambda: _get_source(source))
+    filename = decode_source(source)
+    with open(filename) as f:
+        content = f.read()
+    return {
+        "content": content,
+    }
diff --git a/gdb/python/lib/gdb/dap/threads.py b/gdb/python/lib/gdb/dap/threads.py
index a4ca9e61488..515966761ce 100644
--- a/gdb/python/lib/gdb/dap/threads.py
+++ b/gdb/python/lib/gdb/dap/threads.py
@@ -16,7 +16,6 @@
 import gdb
 
 from .server import request
-from .startup import send_gdb_with_response, in_gdb_thread
 
 
 def _thread_name(thr):
@@ -27,9 +26,8 @@ def _thread_name(thr):
     return None
 
 
-# A helper function to construct the list of threads.
-@in_gdb_thread
-def _get_threads():
+@request("threads")
+def threads(**args):
     result = []
     for thr in gdb.selected_inferior().threads():
         one_result = {
@@ -39,12 +37,6 @@ def _get_threads():
         if name is not None:
             one_result["name"] = name
         result.append(one_result)
-    return result
-
-
-@request("threads")
-def threads(**args):
-    result = send_gdb_with_response(_get_threads)
     return {
         "threads": result,
     }

-- 
2.41.0

[PATCH v2 2/3] Remove ExecutionInvoker

[Tom Tromey]

ExecutionInvoker is no longer really needed, due to the previous DAP
refactoring.  This patch removes it in favor of an ordinary function.
One spot (the 'continue' request) could still have used it, but is
more succinctly expressed as a lambda.
---
 gdb/python/lib/gdb/dap/events.py | 29 +++++++----------------------
 gdb/python/lib/gdb/dap/launch.py |  4 ++--
 gdb/python/lib/gdb/dap/next.py   | 10 +++++-----
 gdb/python/lib/gdb/dap/pause.py  |  4 ++--
 4 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/events.py b/gdb/python/lib/gdb/dap/events.py
index e9ddcab135f..09214ec3dc8 100644
--- a/gdb/python/lib/gdb/dap/events.py
+++ b/gdb/python/lib/gdb/dap/events.py
@@ -17,7 +17,7 @@ import enum
 import gdb
 
 from .server import send_event
-from .startup import in_gdb_thread, Invoker, log
+from .startup import exec_and_log, in_gdb_thread, log
 from .modules import is_module, make_module
 
 
@@ -111,29 +111,14 @@ _expected_stop = None
 
 
 @in_gdb_thread
-def expect_stop(reason):
-    """Indicate that a stop is expected, for the reason given."""
+def exec_and_expect_stop(cmd, reason):
+    """Indicate that a stop is expected, then execute CMD"""
     global _expected_stop
     _expected_stop = reason
-
-
-# A wrapper for Invoker that also sets the expected stop.
-class ExecutionInvoker(Invoker):
-    """A subclass of Invoker that sets the expected stop.
-    Note that this assumes that the command will restart the inferior,
-    so it will also cause ContinuedEvents to be suppressed."""
-
-    def __init__(self, cmd, expected):
-        super().__init__(cmd)
-        self.expected = expected
-
-    @in_gdb_thread
-    def __call__(self):
-        expect_stop(self.expected)
-        global _suppress_cont
-        _suppress_cont = True
-        # FIXME if the call fails should we clear _suppress_cont?
-        super().__call__()
+    global _suppress_cont
+    _suppress_cont = True
+    # FIXME if the call fails should we clear _suppress_cont?
+    exec_and_log(cmd)
 
 
 @in_gdb_thread
diff --git a/gdb/python/lib/gdb/dap/launch.py b/gdb/python/lib/gdb/dap/launch.py
index e81d2849a8e..ab704c7a7cc 100644
--- a/gdb/python/lib/gdb/dap/launch.py
+++ b/gdb/python/lib/gdb/dap/launch.py
@@ -18,7 +18,7 @@ import gdb
 # These are deprecated in 3.9, but required in older versions.
 from typing import Mapping, Optional, Sequence
 
-from .events import ExecutionInvoker
+from .events import exec_and_expect_stop
 from .server import request, capability
 from .startup import in_gdb_thread, exec_and_log
 
@@ -85,4 +85,4 @@ def config_done(**args):
     if _program is not None:
         # Suppress the continue event, but don't set any particular
         # expected stop.
-        ExecutionInvoker("run", None)()
+        exec_and_expect_stop("run", None)
diff --git a/gdb/python/lib/gdb/dap/next.py b/gdb/python/lib/gdb/dap/next.py
index 431020e32e1..eedc26f28a5 100644
--- a/gdb/python/lib/gdb/dap/next.py
+++ b/gdb/python/lib/gdb/dap/next.py
@@ -15,7 +15,7 @@
 
 import gdb
 
-from .events import StopKinds, ExecutionInvoker
+from .events import StopKinds, exec_and_expect_stop
 from .server import capability, request
 from .startup import in_gdb_thread, send_gdb, send_gdb_with_response
 from .state import set_thread
@@ -57,7 +57,7 @@ def next(
     cmd = "next"
     if granularity == "instruction":
         cmd += "i"
-    ExecutionInvoker(cmd, StopKinds.STEP)()
+    exec_and_expect_stop(cmd, StopKinds.STEP)
 
 
 @capability("supportsSteppingGranularity")
@@ -70,13 +70,13 @@ def step_in(
     cmd = "step"
     if granularity == "instruction":
         cmd += "i"
-    ExecutionInvoker(cmd, StopKinds.STEP)()
+    exec_and_expect_stop(cmd, StopKinds.STEP)
 
 
 @request("stepOut", response=False)
 def step_out(*, threadId: int, singleThread: bool = False, **args):
     _handle_thread_step(threadId, singleThread, True)
-    ExecutionInvoker("finish", StopKinds.STEP)()
+    exec_and_expect_stop("finish", StopKinds.STEP)
 
 
 # This is a server-side request because it is funny: it wants to
@@ -87,5 +87,5 @@ def step_out(*, threadId: int, singleThread: bool = False, **args):
 @request("continue", on_dap_thread=True)
 def continue_request(*, threadId: int, singleThread: bool = False, **args):
     locked = send_gdb_with_response(lambda: _handle_thread_step(threadId, singleThread))
-    send_gdb(ExecutionInvoker("continue", None))
+    send_gdb(lambda: exec_and_expect_stop("continue", None))
     return {"allThreadsContinued": not locked}
diff --git a/gdb/python/lib/gdb/dap/pause.py b/gdb/python/lib/gdb/dap/pause.py
index d96172c0757..d276ab1cb92 100644
--- a/gdb/python/lib/gdb/dap/pause.py
+++ b/gdb/python/lib/gdb/dap/pause.py
@@ -13,10 +13,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from .events import StopKinds, ExecutionInvoker
+from .events import StopKinds, exec_and_expect_stop
 from .server import request
 
 
 @request("pause", response=False)
 def pause(**args):
-    ExecutionInvoker("interrupt -a", StopKinds.PAUSE)()
+    exec_and_expect_stop("interrupt -a", StopKinds.PAUSE)

-- 
2.41.0

[PATCH v2 3/3] Implement the notStopped DAP response

[Tom Tromey]

DAP specifies that a request can fail with the "notStopped" message if
the inferior is running but the request requires that it first be
stopped.

This patch implements this for gdb.  Most requests are assumed to
require a stopped inferior, and the exceptions are noted by a new
'request' parameter.

You may notice that the implementation is a bit racy.  I think this is
inherent -- unless the client waits for a stop event before sending a
request, the request may be processed at any time relative to a stop.

https://sourceware.org/bugzilla/show_bug.cgi?id=31037
---
 gdb/python/lib/gdb/dap/events.py | 15 +++++++++++++++
 gdb/python/lib/gdb/dap/pause.py  |  2 +-
 gdb/python/lib/gdb/dap/server.py | 39 ++++++++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.dap/pause.exp  |  7 +++++++
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/events.py b/gdb/python/lib/gdb/dap/events.py
index 09214ec3dc8..bfc3f9ee1dc 100644
--- a/gdb/python/lib/gdb/dap/events.py
+++ b/gdb/python/lib/gdb/dap/events.py
@@ -21,8 +21,17 @@ from .startup import exec_and_log, in_gdb_thread, log
 from .modules import is_module, make_module
 
 
+# True when the inferior is thought to be running, False otherwise.
+# This may be accessed from any thread, which can be racy.  However,
+# this unimportant because this global is only used for the
+# 'notStopped' response, which itself is inherently racy.
+inferior_running = False
+
+
 @in_gdb_thread
 def _on_exit(event):
+    global inferior_running
+    inferior_running = False
     code = 0
     if hasattr(event, "exit_code"):
         code = event.exit_code
@@ -48,6 +57,8 @@ def thread_event(event, reason):
 
 @in_gdb_thread
 def _new_thread(event):
+    global inferior_running
+    inferior_running = True
     thread_event(event, "started")
 
 
@@ -85,6 +96,8 @@ _suppress_cont = False
 
 @in_gdb_thread
 def _cont(event):
+    global inferior_running
+    inferior_running = True
     global _suppress_cont
     if _suppress_cont:
         log("_suppress_cont case")
@@ -123,6 +136,8 @@ def exec_and_expect_stop(cmd, reason):
 
 @in_gdb_thread
 def _on_stop(event):
+    global inferior_running
+    inferior_running = False
     log("entering _on_stop: " + repr(event))
     global _expected_stop
     obj = {
diff --git a/gdb/python/lib/gdb/dap/pause.py b/gdb/python/lib/gdb/dap/pause.py
index d276ab1cb92..b7e21452d69 100644
--- a/gdb/python/lib/gdb/dap/pause.py
+++ b/gdb/python/lib/gdb/dap/pause.py
@@ -17,6 +17,6 @@ from .events import StopKinds, exec_and_expect_stop
 from .server import request
 
 
-@request("pause", response=False)
+@request("pause", response=False, expect_stopped=False)
 def pause(**args):
     exec_and_expect_stop("interrupt -a", StopKinds.PAUSE)
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index 4430d2aba5f..031bf49f486 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -13,6 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import functools
 import inspect
 import json
 import queue
@@ -163,7 +164,28 @@ def send_event(event, body=None):
     _server.send_event(event, body)
 
 
-def request(name: str, *, response: bool = True, on_dap_thread: bool = False):
+# A helper decorator that checks whether the inferior is running.
+def _check_not_running(func):
+    @functools.wraps(func)
+    def check(*args, **kwargs):
+        # Import this as late as possible.  This is done to avoid
+        # circular imports.
+        from .events import inferior_running
+
+        if inferior_running:
+            raise Exception("notStopped")
+        return func(*args, **kwargs)
+
+    return check
+
+
+def request(
+    name: str,
+    *,
+    response: bool = True,
+    on_dap_thread: bool = False,
+    expect_stopped: bool = True
+):
     """A decorator for DAP requests.
 
     This registers the function as the implementation of the DAP
@@ -178,6 +200,11 @@ def request(name: str, *, response: bool = True, on_dap_thread: bool = False):
 
     If ON_DAP_THREAD is True, the function will be invoked in the DAP
     thread.  When ON_DAP_THREAD is True, RESPONSE may not be False.
+
+    If EXPECT_STOPPED is True (the default), then the request will
+    fail with the 'notStopped' reason if it is processed while the
+    inferior is running.  When EXPECT_STOPPED is False, the request
+    will proceed regardless of the inferior's state.
     """
 
     # Validate the parameters.
@@ -217,6 +244,12 @@ def request(name: str, *, response: bool = True, on_dap_thread: bool = False):
 
                 cmd = non_sync_call
 
+        # If needed, check that the inferior is not running.  This
+        # wrapping is done last, so the check is done first, before
+        # trying to dispatch the request to another thread.
+        if expect_stopped:
+            cmd = _check_not_running(cmd)
+
         global _commands
         _commands[name] = cmd
         return cmd
@@ -255,13 +288,13 @@ def initialize(**args):
     return _capabilities.copy()
 
 
-@request("terminate")
+@request("terminate", expect_stopped=False)
 @capability("supportsTerminateRequest")
 def terminate(**args):
     exec_and_log("kill")
 
 
-@request("disconnect", on_dap_thread=True)
+@request("disconnect", on_dap_thread=True, expect_stopped=False)
 @capability("supportTerminateDebuggee")
 def disconnect(*, terminateDebuggee: bool = False, **args):
     if terminateDebuggee:
diff --git a/gdb/testsuite/gdb.dap/pause.exp b/gdb/testsuite/gdb.dap/pause.exp
index 27955d31526..558ede982ee 100644
--- a/gdb/testsuite/gdb.dap/pause.exp
+++ b/gdb/testsuite/gdb.dap/pause.exp
@@ -32,6 +32,13 @@ if {[dap_launch $testfile] == ""} {
 dap_check_request_and_response "start inferior" configurationDone
 dap_wait_for_event_and_check "inferior started" thread "body reason" started
 
+set resp [lindex [dap_request_and_response evaluate {o expression [s 23]}] \
+	      0]
+gdb_assert {[dict get $resp success] == "false"} \
+    "evaluate failed while inferior executing"
+gdb_assert {[dict get $resp message] == "notStopped"} \
+    "evaluate issued notStopped"
+
 dap_check_request_and_response pause pause \
     {o threadId [i 1]}
 

-- 
2.41.0

Re: [PATCH v2 0/3] Implement the DAP notStopped response

[Kévin Le Gouguec]

Tom Tromey <tromey@adacore.com> writes:

> Changes in v2:
> - Changed a few names as suggested by Kévin
> - Removed a FIXME in favor of an explanation

Thanks, looks good to me!

*peeks at latest revision of git trailer patch*

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

Re: [PATCH v2 0/3] Implement the DAP notStopped response

[Tom Tromey]

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

Tom> This series implements the DAP 'notStopped' response.
Tom> Tested on x86-64 Fedora 38.

Tom> This probably conflicts with other DAP patches I've sent, but of
Tom> course I'll rebase & fix up as needed if/when those land.

I did this and re-tested it.  I'm going to check this in now, including
on the gdb-14 branch.

I have just a couple more (known) patches for DAP for gdb-14.
One is the NoOpStructPrinter patch, I'm going to land that soon as well.

The other is a patch in development to fix a buglet I found in the
'pause' request.

Currently I don't plan to try to get the stop-reason series into gdb-14.
Those patches seem more like nice-to-have.

Tom