diff mbox series

[5/6] Catch BaseException in send_gdb_with_response

Message ID 20231201-dap-cancel-v1-5-872022fc328a@adacore.com
State New
Headers
Series Implement DAP cancellation |

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-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey Dec. 1, 2023, 3:41 p.m. UTC 
  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(-)

Comments

Kévin Le Gouguec Dec. 1, 2023, 4:29 p.m. UTC | #1 
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..1a7ec17e5e3 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 BaseException as e:
>              result_q.put(e)
>  
>      send_gdb(message)
>      val = result_q.get()
> -    if isinstance(val, Exception):
> +    if isinstance(val, BaseException):
>          raise val
>      return val

Looking at [the hierarchy], it looks like BaseException will catch more
than we really want?  Should we go for

    except (Exception, KeyboardInterrupt) as e:

    if isinstance(val, (Exception, KeyboardInterrupt)):

?


[the hierarchy]: https://docs.python.org/3/library/exceptions.html#exception-hierarchy
Tom Tromey Dec. 1, 2023, 5:51 p.m. UTC | #2 
Kévin> Looking at [the hierarchy], it looks like BaseException will catch more
Kévin> than we really want?  Should we go for

Kévin>     except (Exception, KeyboardInterrupt) as e:

Kévin>     if isinstance(val, (Exception, KeyboardInterrupt)):

Kévin> ?

Yeah, makes sense.  I've made this change locally.
I also updated the commit message a little to match.

Tom
diff mbox series

Patch

diff --git a/gdb/python/lib/gdb/dap/startup.py b/gdb/python/lib/gdb/dap/startup.py
index a16b51f7cf5..1a7ec17e5e3 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 BaseException as e:
             result_q.put(e)
 
     send_gdb(message)
     val = result_q.get()
-    if isinstance(val, Exception):
+    if isinstance(val, BaseException):
         raise val
     return val