Fix native resource leaks in Keychain#184
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes three native resource leaks in Xamarin.MacDev/Keychain.cs by releasing CoreFoundation/Keychain-allocated memory that was previously not freed.
Changes:
- Release the
CFDataRefreturned bySecCertificateCopyDatainGetAllSigningIdentitiesandGetAllSigningCertificates. - Free
passwordDataviaSecKeychainItemFreeContentand release the item ref viaCFReleaseinFindInternetUserNameAndPasswordandFindInternetPassword(string, ...). - Free
passwordDatainFindInternetPassword(Uri)(the item was already released).
- GetAllSigningIdentities: CFRelease the CFDataRef returned by SecCertificateCopyData (Copy semantics = +1 retain count). - GetAllSigningCertificates: Same CFDataRef leak fix. - FindInternetUserNameAndPassword: Free passwordData with SecKeychainItemFreeContent and release the item ref. - FindInternetPassword(string,...): Same leak fix. - FindInternetPassword(Uri): Free passwordData and align cleanup. All three FindInternet* methods now use try/finally to guarantee native resources are freed even if GetUsernameFromKeychainItemRef or marshaling throws. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rmarinho
force-pushed
the
fix/keychain-native-resource-leaks
branch
from
8e6ffed to
9d01bb0
Compare
rolfbjarne
approved these changes
Jun 1, 2026
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
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.
Summary
Fixes memory leaks in Keychain P/Invoke methods:
GetAllSigningIdentities/GetAllSigningCertificates: TheCFDataRefreturned bySecCertificateCopyData(which follows Copy semantics = +1 retain count) was never released. Now callsCFRelease(data)after extracting bytes.FindInternetUserNameAndPassword/FindInternetPassword(string,...): ThepasswordDataallocated by the keychain was never freed, and the item ref was never released. Now callsSecKeychainItemFreeContentandCFRelease(item).FindInternetPassword(Uri): ThepasswordDatawas never freed (item was already released correctly). Now callsSecKeychainItemFreeContent.These leaks accumulate when enumerating large keychains or repeatedly querying internet passwords.
Testing
No unit tests added — these methods require macOS keychain access with real native interop. Verified the fix compiles correctly and follows the same pattern used in
DoesKeychainContainCertificate(line 616).