Re: [PATCH] Merge handle_inferior_event and handle_inferior_event_1

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Monday, March 25 2019, Tom Tromey wrote:

>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> So it looks like your patch made it so that the value in question
> Pedro> avoids the crash because the value in question is no longer in the
> Pedro> all_values vector by the time gdb is tearing down?
>
> Makes sense.

Thanks Pedro for the explanation.  Yeah, after investigating it became
clear to me that the patch fixes (or "fixes") the bug by destroying the
value instances before we exit GDB.

What's interesting is the fact that I cannot seem to reproduce easily.
For example, if you print anything other than an xmethod, and then you
print an xmethod, and then exit GDB, everything works correctly.  For
that reason, I'm still not able to write a testcase for it, because our
testing infrastructure prints some stuff as a preparation for the test.

> Pedro> Open questions then would be:
>
> Pedro>  - what was the exception in question that was thrown from
> Pedro>    within handle_inferior_event_1?  (And if there was
> Pedro>    no exception, then the theory above is incorrect.)
>
> Pedro>  - why don't non-temporary xmethod values put in the value history
> Pedro>    cause a similar crash at tear down time?
>
> Also, if a crash can occur during value destruction like this, then I
> think it must indicate a bug somewhere else, like invalid reference
> counting somewhere.

That's a good point, yeah.  I can try investigating more.

Having said that, I think the patch is simple enough to be safely
backportable to 8.3, and it doesn't really break anything (in fact, as
Pedro mentioned, it makes things more correct, because now we're sure
that we will destroy the objects if an exception is thrown), so IMHO
there's no downside to it.  WDYT?

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/