Re: [PATCH 2/2] [gdb/dap] Fix stray KeyboardInterrupt after cancel

[Tom Tromey <tom@tromey.com>]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The idea is that this interrupt triggers while in fn here in message (a nested
Tom> function of send_gdb_with_response):
Tom> ...
Tom>     def message():
Tom>         try:
Tom>             val = fn()
Tom>             result_q.put(val)
Tom>         except (Exception, KeyboardInterrupt) as e:
Tom>             result_q.put(e)
Tom> ...
Tom> but instead it triggers outside the try/except.

Thanks for the clear explanation.

Tom> I left the two "sleep 0.2" in the test-case, I didn't see a reason to remove
Tom> those.

Those are there to try to make the test actually interrupt the command
in question -- removing them would make it more likely to just interrupt
the surrounding dispatch machinery, which isn't interesting.

Tom> @@ -435,12 +466,19 @@ def send_gdb_with_response(fn):
 
It seems to me that send_gdb also has this issue -- that is, the
cancellation could arrive just in time to cause a KeyboardInterrupt.

Also I wonder how multi-step / DAP-thread commands like 'continue' are
handled.

Tom>      def message():
Tom>          try:
Tom> -            val = fn()
Tom> +            if not _server.canceller.enter_interruptable():
Tom> +                raise KeyboardInterrupt()
Tom> +            try:
Tom> +                val = fn()
Tom> +            finally:
Tom> +                _server.canceller.exit_interruptable()

This kind of thing is better with a context manager.

Tom>      send_gdb(message)
Tom> +    if _server.canceller.is_cancel_pending():
Tom> +        result_q.put(KeyboardInterrupt())
Tom>      val = result_q.get()

I don't understand how this addition has an effect.  It seems to me that
since a Queue is FIFO, and since the response Queue is only used for a
single value, that this addition will just be ignored.

FWIW, cancellation is inherently racy.  I'm not opposed to trying to
make this more reliable but I think it may be pretty hard.

Maybe some kind of installable exception handler that is run when
post_event fails would be one way.  Not sure.

Tom