[src] Fix memory leaks: release retained handles from Copy/Create native calls#24794
[src] Fix memory leaks: release retained handles from Copy/Create native calls#24794rolfbjarne merged 2 commits intomainfrom
Conversation
…ive calls All these native functions follow Apple's Create/Copy rule, returning retained handles that the caller must release. The handles were being passed to FromHandle/GetNSObject/ArrayFromHandle without specifying releaseHandle: true or owns: true, leaking the native objects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests for APIs modified in the previous commit to verify correct behavior after adding proper handle release semantics. Tests added: - ABAddressBook.LocalizedLabel - ABPerson.LocalizedPropertyName (both overloads) - ABPerson.ToString (via ABRecordCopyCompositeName) - ABPerson.Image/GetImage (no-image case) - ABPerson.GetLinkedPeople - ABPerson.CreateFromVCard - ABRecord.PropertyToString (via FirstName property) - ABMultiValue.Label getter - ABMultiValue.GetValues - CFBundle.GetInfoDictionary (static URL overload) - CFHTTPMessage.GetAllHeaderFields - CGEvent.ToData - CTFont.GetAttribute - UTType.CreateAllIdentifiers - UTType.CopyAllTags - UTType.GetDescription Not directly testable: - CFType.GetDescription: CFType has an internal constructor and cannot be instantiated from the test assembly. The fix is exercised indirectly through other APIs that call ToString() on CF types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request systematically fixes memory leaks throughout the codebase by properly releasing retained handles returned from Apple's Create/Copy native functions. According to Apple's memory management rules, functions with "Create" or "Copy" in their names return retained handles that the caller is responsible for releasing. The PR adds releaseHandle: true or owns: true parameters to handle conversion methods (CFString.FromHandle, Runtime.GetNSObject, NSArray.ArrayFromHandle, CFArray.StringArrayFromHandle) when processing handles from these functions. Comprehensive tests have been added for the affected APIs to ensure correct behavior.
Changes:
- Fixed memory leaks in 13 source files by properly releasing retained handles from Create/Copy native functions
- Added tests for 7 previously untested APIs to ensure the fixes work correctly
- Covered multiple frameworks: MobileCoreServices, CoreVideo, CoreText, CoreMedia, CoreGraphics, CoreFoundation, CFNetwork, and AddressBook
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/MobileCoreServices/UTType.cs | Fixed memory leaks in UTTypeCreatePreferredIdentifierForTag, UTTypeCreateAllIdentifiersForTag, UTTypeCopyAllTagsWithClass, UTTypeCopyDescription, UTTypeCopyPreferredTagWithClass, UTTypeCopyDeclaration, UTTypeCopyDeclaringBundleURL |
| src/CoreVideo/CVPixelFormatDescription.cs | Fixed memory leak in CVPixelFormatDescriptionCreateWithPixelFormatType |
| src/CoreText/CTFont.cs | Fixed memory leak in CTFontCopyAttribute |
| src/CoreMedia/CMTime.cs | Fixed memory leak in CMTimeCopyDescription |
| src/CoreGraphics/CGEvent.cs | Fixed memory leak in CGEventCreateData |
| src/CoreFoundation/CFType.cs | Fixed memory leak in CFCopyDescription |
| src/CoreFoundation/CFBundle.cs | Fixed memory leak in CFBundleCopyInfoDictionaryForURL and removed misleading comment |
| src/CFNetwork/CFHTTPMessage.cs | Fixed memory leak in CFHTTPMessageCopyAllHeaderFields |
| src/AddressBook/ABRecord.cs | Fixed memory leaks in ABRecordCopyCompositeName and CopyValue helper |
| src/AddressBook/ABPerson.cs | Fixed memory leaks in ABPersonCopyLocalizedPropertyName (both overloads), ABPersonCopyImageData, ABPersonCopyArrayOfAllLinkedPeople, ABPersonCopyImageDataWithFormat, ABPersonCreatePeopleInSourceWithVCardRepresentation |
| src/AddressBook/ABMultiValue.cs | Fixed memory leaks in CopyLabelAtIndex and CopyArrayOfAllValues |
| src/AddressBook/ABAddressBook.cs | Fixed memory leaks in ABAddressBookCopyLocalizedLabel and ABAddressBookCopyDefaultSource |
| tests/monotouch-test/MobileCoreServices/UTTypeTest.cs | Added tests for CreateAllIdentifiers, CopyAllTags, and GetDescription |
| tests/monotouch-test/CoreText/FontTest.cs | Added test for GetAttribute |
| tests/monotouch-test/CoreServices/HttpMessageTest.cs | Added test for GetAllHeaderFields |
| tests/monotouch-test/CoreGraphics/CGEventTests.cs | Added test for ToData |
| tests/monotouch-test/CoreFoundation/BundleTest.cs | Added test for GetInfoDictionary |
| tests/monotouch-test/AddressBook/PersonTest.cs | Added tests for LocalizedPropertyName (both overloads), PersonToString, GetImage_NoImage, GetLinkedPeople, CreateFromVCard, PropertyToString_FirstName, MultiValueLabel, MultiValueGetValues |
| tests/monotouch-test/AddressBook/AddressBookTest.cs | Added test for LocalizedLabel |
✅ [CI Build #e9091d7] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #e9091d7] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #e9091d7] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #e9091d7] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 156 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
All these native functions follow Apple's Create/Copy rule, returning retained
handles that the caller must release. The handles were being passed to
FromHandle/GetNSObject/ArrayFromHandle without specifying releaseHandle: true
or owns: true, leaking the native objects.
Also add tests for those APIs that were changed but didn't have tests already.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com