[lldb][test] Disable parallel module loading for TestNetBSDCore.py #159395
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
Since llvm#157170 this test has been flakey on several LLDB buildbots. I suspect it's to do with mutli-threading, there are more details in llvm#159377. Disable parallel loading for now so we are not spamming people making unrelated changes.
DavidSpickett
added
the
skip-precommit-approval
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) ChangesSince #157170 this test has been flakey on several LLDB buildbots. I suspect it's to do with mutli-threading, there are more details in #159377. Disable parallel loading for now so we are not spamming people making unrelated changes. Full diff: https://github.com/llvm/llvm-project/pull/159395.diff 1 Files Affected:
diff --git a/lldb/test/API/functionalities/postmortem/netbsd-core/TestNetBSDCore.py b/lldb/test/API/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
index ff1ef21e02e31..179dbdf88fa8a 100644
--- a/lldb/test/API/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
+++ b/lldb/test/API/functionalities/postmortem/netbsd-core/TestNetBSDCore.py
@@ -117,6 +117,8 @@ def check_backtrace(self, thread, filename, backtrace):
)
def do_test(self, filename, pid, region_count):
+ # Temporary workaround for https://github.com/llvm/llvm-project/issues/159377
+ self.runCmd("settings set target.parallel-module-load false")
target = self.dbg.CreateTarget(filename)
process = target.LoadCore(filename + ".core")
|
dmpots
added a commit
to dmpots/llvm-project
that referenced
this pull request
Sep 17, 2025
The `ProcessElfCore::FindModuleUUID` function can be called by multiple
threads at the same time when `target.parallel-module-load` is true. We
were using the `operator[]` to lookup the UUID which will mutate the map
when the key is not present. This is unsafe in a multi-threaded contex
so we now use a read-only `find` operation and explicitly return an
invalid UUID when the key is not present.
The `m_uuids` map can follow a create-then-query pattern. It is
populated in the `DoLoadCore` function which looks like it is only
called in a single-threaded context so we do not need extra locking as
long as we keep the other accesses read-only.
Other fixes I considered
* Use a lock to protect access
- We don't need to modify the map after creation so we can allow
concurrent read-only access.
* Store the map in a const pointer container to prevent accidental
mutation in other places.
- Only accessed in one place currently so just added a comment.
* Store the UUID in the NT_FILE_Entry after building the mapping
correctly in `UpdateBuildIdForNTFileEntries`.
- The map lets us avoid a linear search in `FindModuleUUID`.
This commit also reverts the temporary workaround from llvm#159395 which
disabled parallel module loading to avoid the test failure.
dmpots
added a commit
that referenced
this pull request
Sep 18, 2025
…9444) The `ProcessElfCore::FindModuleUUID` function can be called by multiple threads at the same time when `target.parallel-module-load` is true. We were using the `operator[]` to lookup the UUID which will mutate the map when the key is not present. This is unsafe in a multi-threaded contex so we now use a read-only `find` operation and explicitly return an invalid UUID when the key is not present. The `m_uuids` map can follow a create-then-query pattern. It is populated in the `DoLoadCore` function which looks like it is only called in a single-threaded context so we do not need extra locking as long as we keep the other accesses read-only. Other fixes I considered * Use a lock to protect access - We don't need to modify the map after creation so we can allow concurrent read-only access. * Store the map in a const pointer container to prevent accidental mutation in other places. - Only accessed in one place currently so just added a comment. * Store the UUID in the NT_FILE_Entry after building the mapping correctly in `UpdateBuildIdForNTFileEntries`. - The map lets us avoid a linear search in `FindModuleUUID`. This commit also reverts the temporary workaround from #159395 which disabled parallel module loading to avoid the test failure. Fixes #159377
kimsh02
pushed a commit
to kimsh02/llvm-project
that referenced
this pull request
Sep 19, 2025
…lvm#159395) Since llvm#157170 this test has been flakey on several LLDB buildbots. I suspect it's to do with mutli-threading, there are more details in llvm#159377. Disable parallel loading for now so we are not spamming people making unrelated changes.
kimsh02
pushed a commit
to kimsh02/llvm-project
that referenced
this pull request
Sep 19, 2025
…m#159444) The `ProcessElfCore::FindModuleUUID` function can be called by multiple threads at the same time when `target.parallel-module-load` is true. We were using the `operator[]` to lookup the UUID which will mutate the map when the key is not present. This is unsafe in a multi-threaded contex so we now use a read-only `find` operation and explicitly return an invalid UUID when the key is not present. The `m_uuids` map can follow a create-then-query pattern. It is populated in the `DoLoadCore` function which looks like it is only called in a single-threaded context so we do not need extra locking as long as we keep the other accesses read-only. Other fixes I considered * Use a lock to protect access - We don't need to modify the map after creation so we can allow concurrent read-only access. * Store the map in a const pointer container to prevent accidental mutation in other places. - Only accessed in one place currently so just added a comment. * Store the UUID in the NT_FILE_Entry after building the mapping correctly in `UpdateBuildIdForNTFileEntries`. - The map lets us avoid a linear search in `FindModuleUUID`. This commit also reverts the temporary workaround from llvm#159395 which disabled parallel module loading to avoid the test failure. Fixes llvm#159377
SeongjaeP
pushed a commit
to SeongjaeP/llvm-project
that referenced
this pull request
Sep 23, 2025
…lvm#159395) Since llvm#157170 this test has been flakey on several LLDB buildbots. I suspect it's to do with mutli-threading, there are more details in llvm#159377. Disable parallel loading for now so we are not spamming people making unrelated changes.
SeongjaeP
pushed a commit
to SeongjaeP/llvm-project
that referenced
this pull request
Sep 23, 2025
…m#159444) The `ProcessElfCore::FindModuleUUID` function can be called by multiple threads at the same time when `target.parallel-module-load` is true. We were using the `operator[]` to lookup the UUID which will mutate the map when the key is not present. This is unsafe in a multi-threaded contex so we now use a read-only `find` operation and explicitly return an invalid UUID when the key is not present. The `m_uuids` map can follow a create-then-query pattern. It is populated in the `DoLoadCore` function which looks like it is only called in a single-threaded context so we do not need extra locking as long as we keep the other accesses read-only. Other fixes I considered * Use a lock to protect access - We don't need to modify the map after creation so we can allow concurrent read-only access. * Store the map in a const pointer container to prevent accidental mutation in other places. - Only accessed in one place currently so just added a comment. * Store the UUID in the NT_FILE_Entry after building the mapping correctly in `UpdateBuildIdForNTFileEntries`. - The map lets us avoid a linear search in `FindModuleUUID`. This commit also reverts the temporary workaround from llvm#159395 which disabled parallel module loading to avoid the test failure. Fixes llvm#159377
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.
Since #157170 this test has been flakey on several LLDB buildbots. I suspect it's to do with mutli-threading, there are more details in #159377.
Disable parallel loading for now so we are not spamming people making unrelated changes.