[lldb][test] Don't call SBDebugger::Terminate if TestMultipleDebuggers times out #143732
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s times out Fixes llvm#101162 This test did this: * SBDebugger::Initialize * Spawn a bunch of threads that do * SBDebugger::Create * some work * SBDebugger::Destroy * Wait on those threads to finish then call SBDebugger::Terminate, or - * Reach a time limit before all the threads finish, call SBDebugger::Terminate and exit. The problem was that in the timeout case, calling SBDebugger::Terminate destroys data being used by threads that are still running. This test was expecting said threads to be so broken they were probably stuck, but when the machine is just heavily loaded, one of them might read that data before the whole program can exit. This means what should have been a timeout is now a crash. Sometimes. Which explains why we saw both timeouts and various signals on the AArch64 Linux bot. It depends on the timings. So I'm changing it not to call SBDebugger::Terminate in the timeout case. We will have to tweak the timeout value based on what happens on the buildbot.
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) ChangesFixes #101162 This test did this:
The problem was that in the timeout case, calling SBDebugger::Terminate destroys data being used by threads that are still running. I expect this test was expecting said threads to be so broken they were probably stuck, but when the machine is just heavily loaded, one of them might read that data before the whole program exits. This means what should have been a timeout becomes a crash. Sometimes. Which explains why we saw both timeouts and various signals on the AArch64 Linux bot. It depends on the timings. So I'm changing it not to call SBDebugger::Terminate in the timeout case. We will have to tweak the timeout value based on what happens on the buildbot, but we will know it's machine load not an lldb bug. Full diff: https://github.com/llvm/llvm-project/pull/143732.diff 2 Files Affected:
diff --git a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
index 1fd4806cd74f4..f0a3893f53aab 100644
--- a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
+++ b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
@@ -12,8 +12,6 @@
class TestMultipleSimultaneousDebuggers(TestBase):
NO_DEBUG_INFO_TESTCASE = True
- # Sometimes times out on Linux, see https://github.com/llvm/llvm-project/issues/101162.
- @skipIfLinux
@skipIfNoSBHeaders
@skipIfWindows
@skipIfHostIncompatibleWithTarget
diff --git a/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp b/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
index 64728fb7c29a1..fcec9bae0ed9c 100644
--- a/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
+++ b/lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
@@ -296,6 +296,8 @@ int main (int argc, char **argv)
NUMBER_OF_SIMULTANEOUS_DEBUG_SESSIONS);
}
- SBDebugger::Terminate();
+ // We do not call SBDebugger::Terminate() here because it will destroy
+ // data that might be being used by threads that are still running. Which
+ // would change the timeout into an unrelated crash.
exit (1);
}
|
|
Side note: this test doesn't actually care if all the threads are successful in what they do, but it's been that way forever and I don't want to get any more involved than I have to :) |
|
This makes the assumption that child threads are torn down before their parent, which might be a faulty one. I looked for a way to cancel std::threads but it seems like you have to get a native handle to it and use the specific OS APIs, which I'd rather not complicate the test with. |
labath
approved these changes
Jun 12, 2025
|
Then what I'm doing is narrowing the window in which crashes could happen, but not closing it. _exit seems more safe for the timeout situation. If there's a genuine bug then yes anything could happen but at least we'd be rewarded for looking into it when it did. If there's still flakiness, I'll declare this impossible to get 100% right and disable it again. I did think I could send a signal from the main thread, but other threads continue running so that doesn't work. Unless it's SIGSTOP, but you can't catch that, and I don't know what return code we'd end up with. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/19295 Here is the relevant piece of the build log for the reference |
|
Timed out on the very first build 🤣 On second thought this test is better disabled. It's more correct than it was, but it's never going to be truly stable. |
tomtor
pushed a commit
to tomtor/llvm-project
that referenced
this pull request
Jun 14, 2025
…s times out (llvm#143732) Fixes llvm#101162 This test did this: * SBDebugger::Initialize * Spawn a bunch of threads that do: * SBDebugger::Create * some work * SBDebugger::Destroy * Wait on those threads to finish then call SBDebugger::Terminate and exit, or - * Reach a time limit before all the threads finish, call SBDebugger::Terminate and exit. The problem was that in the timeout case, calling SBDebugger::Terminate destroys data being used by threads that are still running. I expect this test was expecting said threads to be so broken they were probably stuck, but when the machine is just heavily loaded, one of them might read that data before the whole program exits. This means what should have been a timeout becomes a crash. Sometimes. Which explains why we saw both timeouts and various signals on the AArch64 Linux bot. It depends on the timings. So I'm changing it not to call SBDebugger::Terminate in the timeout case. We will have to tweak the timeout value based on what happens on the buildbot, but we will know it's machine load not an lldb bug. Also use _exit instead of exit, to skip more cleanup that might cause a crash.
akuhlens
pushed a commit
to akuhlens/llvm-project
that referenced
this pull request
Jun 24, 2025
…s times out (llvm#143732) Fixes llvm#101162 This test did this: * SBDebugger::Initialize * Spawn a bunch of threads that do: * SBDebugger::Create * some work * SBDebugger::Destroy * Wait on those threads to finish then call SBDebugger::Terminate and exit, or - * Reach a time limit before all the threads finish, call SBDebugger::Terminate and exit. The problem was that in the timeout case, calling SBDebugger::Terminate destroys data being used by threads that are still running. I expect this test was expecting said threads to be so broken they were probably stuck, but when the machine is just heavily loaded, one of them might read that data before the whole program exits. This means what should have been a timeout becomes a crash. Sometimes. Which explains why we saw both timeouts and various signals on the AArch64 Linux bot. It depends on the timings. So I'm changing it not to call SBDebugger::Terminate in the timeout case. We will have to tweak the timeout value based on what happens on the buildbot, but we will know it's machine load not an lldb bug. Also use _exit instead of exit, to skip more cleanup that might cause a crash.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #101162
This test did this:
The problem was that in the timeout case, calling SBDebugger::Terminate destroys data being used by threads that are still running. I expect this test was expecting said threads to be so broken they were probably stuck, but when the machine is just heavily loaded, one of them might read that data before the whole program exits.
This means what should have been a timeout becomes a crash. Sometimes. Which explains why we saw both timeouts and various signals on the AArch64 Linux bot. It depends on the timings.
So I'm changing it not to call SBDebugger::Terminate in the timeout case. We will have to tweak the timeout value based on what happens on the buildbot, but we will know it's machine load not an lldb bug.
Also use _exit instead of exit, to skip more cleanup that might cause a crash.