Re: [PATCH 2/3] Fix crash if connection drops in scoped_restore_current_thread's ctor, part 2

[Simon Marchi <simark@simark.ca>]

On 2020-07-08 7:31 p.m., Pedro Alves wrote:
> Running the testsuite against an Asan-enabled build of GDB makes
> gdb.base/multi-target.exp expose this bug.
> 
> scoped_restore_current_thread's ctor calls get_frame_id to record the
> selected frame's ID to restore later.  If the frame ID hasn't been
> computed yet, it will be computed on the spot, and that will usually
> require accessing the target's memory and registers.  If the remote
> connection closes, while we're computing the frame ID, the remote
> target exits its inferiors, unpushes itself, and throws a
> TARGET_CLOSE_ERROR error.  Exiting the inferiors deletes the
> inferior's threads.
> 
> scoped_restore_current_thread increments the current thread's refcount
> to prevent the thread from being deleted from under its feet.
> However, the code that does that isn't considering the case of the
> thread being deleted from within get_frame_id.  It only increments the
> refcount _after_ get_frame_id returns.  So if the current thread is
> indeed deleted, the
> 
>      tp->incref ();
> 
> statement references a stale TP pointer.
> 
> Incrementing the refcounts earlier fixes it.
> 
> We should probably also let the TARGET_CLOSE_ERROR error propagate in
> this case.  That alone would fix it, though it seems better to tweak
> the refcount handling too.

So, when the target closes while we (scoped_restore_current_thread) own
a reference on the inferior and thread, the inferior and thread are still
destroyed, and so we shouldn't decref them?

Simon