[Lldb-commits] [PATCH] D93479: [lldb] Simplify the is_finalized logic in process and make it thread safe.
Adam Brouwers-Harries via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Jun 25 04:30:30 PDT 2021
aharries-upmem added a comment.
In D93479#2839076 <https://reviews.llvm.org/D93479#2839076>, @jingham wrote:
> I wonder if instead of doing:
>
> // Use our target to get a shared pointer to ourselves...
> if (m_finalize_called && !PrivateStateThreadIsValid())
> BroadcastEvent(event_sp);
> else
> m_private_state_broadcaster.BroadcastEvent(event_sp);
>
> ->
>
> m_private_state_broadcaster.BroadcastEvent(event_sp);
>
> we should have just replaced m_finalize_called with m_finalizing? If you tried to sent the exited event to the private event broadcaster after it was shut down, that event would never get to the public process event queue.
Hi Jim, I've tried reverting this check and replacing `m_finalize_called` with `m_finalizing`, but sadly the code still seems to deadlock.
I'm also a little suspicious of (in `Process::Finalize`):
if (m_finalizing.exchange(true))
return;
rather than
m_finalize_called = true;
As it would seem (to me) to introduce the possibility of an early exit where there wasn't one before. I don't know if that was intended (to avoid things being done twice?), but it's the only other place I can see a clear change in control flow.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93479/new/
https://reviews.llvm.org/D93479
More information about the lldb-commits
mailing list