[Lldb-commits] [PATCH] D93479: [lldb] Simplify the is_finalized logic in process and make it thread safe.

[Jim Ingham via lldb-commits]

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.

That's the only part of the patch that seems a little suspect to me.

Can you try making that change and see if things go better?

Jim


> On Jun 24, 2021, at 6:36 AM, Adam Brouwers-Harries via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> aharries-upmem added a comment.
> 
> Hi all,
> 
> Apologies for being the bearer of bad news, but I believe that this patch breaks our[1] (downstream) lldb, by introducing a deadlock when a process is killed by a parent debugging process. Specifically, I believe that this patch causes a process to fail to exit, which causes a later deadlock when a listener waits for the process to exit.
> 
> I'm in the process of trying to produce some convincing output from our backend so that I can properly file a bug, but in the meantime I wanted to raise this here in case anyone has any comments or thoughts on this. Although I managed to narrow the issue down to this commit through a `git bisect`, I am not confident in this patch being the *cause* of the deadlock, as it may be the case that we are relying on incorrect behaviour in lldb that this patch fixes.
> 
> [1]: https://github.com/upmem/llvm-project/tree/upmem_release_120
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D93479/new/
> 
> https://reviews.llvm.org/D93479
>