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

[Tom Tromey <tom@tromey.com>]

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

Tom> Fix this by:
Tom> - in CancellationHandler, renaming variable in_flight to in_flight_dap_thread,
Tom>   and adding a variable in_flight_gdb_thread to be able to distinguish when
Tom>   a request is in flight in the dap thread or the gdb thread.
Tom> - adding a wrapper Cancellable to to deal with cancelling the wrapped
Tom>   event
Tom> - using Cancellable in send_gdb and send_gdb_with_response to wrap the posted
Tom>   event
Tom> - in CancellationHandler.cancel, only call gdb.interrupt if
Tom>   req == self.in_flight_gdb_thread.

Thanks.  I like this patch a lot.

I have one question about it.

Tom> +    @contextmanager
Tom> +    def interruptable_region(self, req):
Tom> +        with self.lock:
Tom> +            # If the request is cancelled, don't execute the region.
Tom> +            while len(self.reqs) > 0 and self.reqs[0] <= req:
Tom> +                if heapq.heappop(self.reqs) == req:
Tom> +                    raise KeyboardInterrupt()
Tom> +            # Request is being handled by the gdb thread.
Tom> +            self.in_flight_gdb_thread = req

(see below)

Tom> +        try:
Tom> +            # Execute region.  This may be interrupted by gdb.interrupt.
Tom> +            yield None
Tom> +        finally:
Tom> +            # Request has been handled by the gdb thread,
Tom> +            with self.lock:
Tom> +                self.in_flight_gdb_thread = None
Tom> +

I think it's possible for the interrupt to occur at the marked spot
above -- in_flight_gdb_thread has been set and the lock has been
released.

However, in this case it seems like in_flight_gdb_thread won't be reset?

I'm not really sure if this matters.  However, wouldn't moving the 'with'
block inside the 'try' definitely fix this?

Tom