refactor(mach headers): refactor to linked list to avoid threading issues #728
Conversation
This file contains 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
The layout of the MachHeaders module is not optimal as it exposes unnecessary implementation detail. This means the index of the image in the list is passed around and increases the chances of this changing if used in multi-threaded context.
Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.m
Outdated
Show resolved
Hide resolved
Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.m
Outdated
Show resolved
Hide resolved
fractalwrench
approved these changes
Jun 29, 2020
fractalwrench
approved these changes
Jun 29, 2020
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.
Goal
The current implementation of the Mach Headers cache using an expandable array is not easy to make work in a thread-safe way, so wouldn't be usable if threads were not suspended before accessing.
Design
Refactored to a linked list so that there is no
reallocand less change when new elements are added. Also changed removal so the library is simply marked as removed rather than deleted so that locking is not required for this operation.Changeset
BSG_KSMachHeaders- amended implementation to a linked list; renamed functions for internal consistency; amended removal to simply mark the library as removed rather than mutate the list and so removed the lock around the operation; refactored lock functions to work in a cleaner way.BSG_KSCrash- moved initialisation code toBSG_KSCrashC, closer to other initialisation code and moved the registering of changes into the initialisation function of the mach header code.BSG_KSDynamicLinker- moved several functions toBSG_KSMachHeadersto avoid passing indexes around.Tests
freecall during initialization so that the list of headers doesn't grow between each test.