[RGen] When a native string is created, released as part of the post invocation steps. #23029
Conversation
…invocation steps. Instead than calling the GC.KeepAlive method we want to call CFString.Release native which keeps a ref to the native string and, more importantly, frees the memory of the string once we are done with it.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the string release mechanism by replacing GC.KeepAlive with CFString.ReleaseNative in the post invocation steps. Key changes include:
- Updating tests to verify the use of CFString.ReleaseNative.
- Modifying BindingSyntaxFactory.Trampoline.cs to call the new StringReleaseNative method.
- Adding a new StringReleaseNative implementation and updating documentation in BindingSyntaxFactory.Runtime.cs.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryTrampolineTests.cs | Updated test expectations to use CFString.ReleaseNative. |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Trampoline.cs | Replaced GC.KeepAlive with StringReleaseNative in trampoline generation. |
| src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Runtime.cs | Added StringReleaseNative implementation and updated corresponding documentation. |
Comments suppressed due to low confidence (1)
src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Runtime.cs:243
- The documentation for NStringCreateNative incorrectly references 'NFString'. Update it to 'NString' for consistency with the method name.
/// Generates the expression to call the NFString.CreateNative method.
✅ [CI Build #8695ff1] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #ca0b7d6] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commit.NET ( No breaking changes )✅ API diff vs stable.NET ( No breaking changes )ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #8695ff1] 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 #8695ff1] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. 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 #8695ff1] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #8695ff1] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #8695ff1] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #8695ff1] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 115 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
Instead than calling the GC.KeepAlive method we want to call CFString.Release native which keeps a ref to the native string and, more importantly, frees the memory of the string once we are done with it.