[CoreFoundation] Fix numerous issues with CFMessagePort. - #26513
[CoreFoundation] Fix numerous issues with CFMessagePort.#26513rolfbjarne wants to merge 7 commits into
Conversation
* Use the native CFMessagePort context retain/release contract to keep callback state alive * Fix returned object ownership * Make invalidation callbacks replaceable. * Add complete API documentation, a result-returning TrySetName API * Add test coverage. Contributes towards #10146. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 99900f2c-d178-4e70-9068-c6c3005ef89a
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the CoreFoundation.CFMessagePort managed wrapper to be safer and more correct with respect to CoreFoundation context lifetime, returned object ownership, invalidation callback behavior, and API documentation, along with adding regression tests and updating handle-safety known failures.
Changes:
- Reworked local port callback/context lifetime management to rely on CFMessagePort’s retain/release context contract, and adjusted invalidation callback handling to be replaceable.
- Fixed ownership semantics for
SendRequest’s returned data andCreateRunLoopSource’s returned source. - Added new NUnit coverage for naming/invalidation/send-request/run loop source ownership scenarios, and adjusted Cecil handle-safety known failures accordingly.
File summaries
| File | Description |
|---|---|
| src/CoreFoundation/CFMessagePort.cs | Core wrapper changes: context retain/release, invalidation callback behavior, ownership fixes, and expanded XML docs. |
| tests/monotouch-test/CoreFoundation/CFMessagePortTest.cs | New end-to-end tests validating callback replacement, duplicate port callback behavior, send request semantics, and run loop source ownership. |
| tests/cecil-tests/HandleSafety.KnownFailures.cs | Updates the known-failures list to reflect removed/changed implementation details in CFMessagePort. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Remove the null-forgiving operator at the unmanaged callback boundary and return no reply if Core Foundation unexpectedly supplies a null data object. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 99900f2c-d178-4e70-9068-c6c3005ef89a
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
✅ Applied expected app size files from gist. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
✅ Applied expected app size files from gist. |
✅ 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 #dc916b6] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 264 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Contributes towards #10146.
Copilot-Session: 99900f2c-d178-4e70-9068-c6c3005ef89a