[Lldb-commits] [PATCH] D98822: [lldb] [Process] Watch for fork/vfork notifications

Tue Apr 13 05:30:05 PDT 2021

labath added a comment.

In D98822#2685256 <https://reviews.llvm.org/D98822#2685256>, @mgorny wrote:

> In D98822#2685246 <https://reviews.llvm.org/D98822#2685246>, @labath wrote:
>
>> I've reverted this (121cff78a8032a73aa4fb820625dc1ecae8e3997 <https://reviews.llvm.org/rG121cff78a8032a73aa4fb820625dc1ecae8e3997>) because it made the linux bot super-flaky. I think it's likely that this affects _only_ linux (and the BSD code is fine -- though we don't have a bot to verify that), but I didn't want to experiment with partial reverts while the bots are wonky. However, it may be interesting the re-land the linux support in a separate commit, once we figure out the problem -- my money is on the nondeterminism of thread creation.
>
> How about recommitting without the `clone()` change, i.e. assuming that `clone` always means thread, and seeing if that causes trouble still?

I think I have it figured out -- see the inlined comment. It seems this was one of those (rare) cases where enabling logging actually makes a race _more_ likely...

================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:904-905
+      NativeThreadLinux &child_thread = AddThread(child_pid, /*resume*/ true);
+      // Resume the newly created thread.
+      ResumeThread(child_thread, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
+      ThreadWasCreated(child_thread);
----------------
This is resuming the thread a second time. If the thread manages to stop between the resume inside AddThread, and here, we will miss the breakpoint (and subsequently crash).

Deleting these two lines should fix things.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98822/new/

https://reviews.llvm.org/D98822