[lldb] Use weak pointers instead of shared pointers in DynamicLoader #156446
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
DynamicLoaderWindowsDYLD uses pointers to Modules to maintain a map
from modules to their addresses, but it does not need to keep "strong"
references to them. Weak pointers should be enough, and would allow
modules to be released elsewhere.
Other DynamicLoader classes do not use shared pointers as well. For
example, DynamicLoaderPOSIXDYLD has a similar map with weak pointers.
Actually testing for modules being completely released can be
tricky. The test here is just to illustrate the case where shared
pointers kept modules in DynamicLoaderWindowsDYLD and prevented them
from being released. The test executes the following sequence:
1. Create a target, load an executable and run it.
2. Remove one module from the target. The target should be the last
actual use of the module, but we have another reference to it
in the shared module cache.
3. Call MemoryPressureDetected to remove this last reference from
the cache.
4. Replace the corresponding DLL file.
LLDB memory maps DLLs, and this makes the files read-only on
Windows. Unless the modules are completely released (and therefore
unmapped), (4) is going to fail with "access denied".
However, the test does not trigger the bug completely - it passes with
and without the change.
Collaborator
Author
|
@jimingham, @JDevlieghere, thanks to your suggestions in #147289, I was able to use existing LLDB API to completely release some modules from the cache, but keep other modules intact. However, this approach did not work completely, because |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
DynamicLoaderWindowsDYLD uses pointers to mModules to maintain a map from modules to their addresses, but it does not need to keep "strong" references to them. Weak pointers should be enough, and would allow modules to be released elsewhere.
Other DynamicLoader classes do not use shared pointers as well. For example, DynamicLoaderPOSIXDYLD has a similar map with weak pointers.
Actually testing for modules being completely released can be tricky. The test here is just to illustrate the case where shared pointers kept modules in DynamicLoaderWindowsDYLD and prevented them from being released. The test executes the following sequence:
Create a target, load an executable and run it.
Remove one module from the target. The target should be the last actual use of the module, but we have another reference to it in the shared module cache.
Call MemoryPressureDetected to remove this last reference from the cache.
Replace the corresponding DLL file.
LLDB memory maps DLLs, and this makes files read-only on Windows. Unless the modules are completely released (and therefore unmapped), (4) is going to fail with "access denied".
However, the test does not trigger the bug completely - it passes with and without the change.