Fix unsoundness in new_library_with_data
#309
Merged
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
|
Thanks for the quick fix! |
|
@madsmtm how about adding a comment to the code with your findings? Specifically, explain above |
The `library_data` slice passed to the function that the dispatch data object is referencing does not necessarily outlive the library in which it will be contained. (Note: It _looks_ like we're upholding memory management rules here, as the object returned from `dispatch_data_create` is released with `dispatch_release` before the end of the function, but remember that the dispatch data is a reference-counted object; `MTLLibrary` will retain the dispatch data beyond the lifetime of the function). As specified in https://developer.apple.com/documentation/dispatch/1452970-dispatch_data_create, if we use DISPATCH_DATA_DESTRUCTOR_DEFAULT as the destructor block instead, `dispatch` will copy the buffer for us automatically.
madsmtm
force-pushed
the
fix-new-library-with-data
branch
from
ad22305
to
b35479e
Compare
MarijnS95
approved these changes
Apr 23, 2024
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.
The
library_dataslice passed to the function that the dispatch data object is referencing does not necessarily outlive the library in which it will be contained.(Note: It looks like we're upholding memory management rules here, as the object returned from
dispatch_data_createis released withdispatch_releasebefore the end of the function, but remember that the dispatch data is a reference-counted object;MTLLibrarywill retain the dispatch data beyond the lifetime of the function).As specified in https://developer.apple.com/documentation/dispatch/1452970-dispatch_data_create, if we use DISPATCH_DATA_DESTRUCTOR_DEFAULT as the destructor block instead,
dispatchwill copy the buffer for us automatically.Fixes #308.