Repository navigation
[Network] Correct Ethernet channel state callbacks. Fixes #26789 - #26871
rolfbjarne wants to merge 3 commits into
Conversation
Use the Ethernet channel state enum in the public callback signature so the block target matches the native trampoline. Add macOS coverage that starts a channel on a wired interface and checks that a native state callback delivers an Ethernet channel state when the custom-protocol entitlement is available. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Describe when the correctly typed state-change callback runs, the Ethernet state and optional error it receives, and how passing null clears the handler. Keep the deprecated compatibility overload and all behavior unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use task completion sources for path and state callbacks so asynchronous notifications cannot signal disposed events during teardown. Capture only the first callback result and cancel the path monitor even when its wait fails. Include the component-model namespace required by the legacy overload visibility attribute. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The runtime regression test is unconditionally skipped in CI, leaving the failing callback path untested.
2 open findings
What changed in this PR
Corrects Ethernet channel callbacks to use NWEthernetChannelState while preserving compatibility.
Changes:
- Adds the correctly typed overload and legacy forwarding.
- Updates XML documentation tracking.
- Adds macOS callback regression coverage.
| File | Description |
|---|---|
src/Network/NWEthernetChannel.cs |
Corrects callback typing and preserves the legacy API. |
tests/monotouch-test/Network/NWEthernetChannelTest.cs |
Tests corrected and legacy callbacks. |
tests/cecil-tests/Documentation.KnownFailures.txt |
Updates documentation expectations. |
🧠 Review effort: Balanced
| public void StateChangesHandler (bool useLegacyHandler) | ||
| { | ||
| TestRuntime.AssertXcodeVersion (11, 0); | ||
| TestRuntime.IgnoreInCI ("This test requires a wired interface and the custom-protocol entitlement."); |
| #if !XAMCORE_5_0 | ||
| [BindingImpl (BindingImplOptions.Optimizable)] | ||
| [EditorBrowsable (EditorBrowsableState.Never)] | ||
| [Obsolete ("Use the other overload.")] |
✅ 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 #5c694a9] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 2 tests failed, 219 tests passed. Failures❌ linker tests (iOS)1 tests failed, 20 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (iOS)1 tests failed, 19 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ 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 |
Add a SetStateChangesHandler overload accepting
Action<NWEthernetChannelState, NWError?>so the registered delegate matches the native Ethernet channel callback and its trampoline.Preserve the existing NWBrowserState overload behind !XAMCORE_5_0, mark it obsolete and hidden, and assign OverloadResolutionPriority (-1) so implicit callbacks select the correctly typed overload. Forward legacy handlers through the new overload, retaining the native numeric state without a delegate-type mismatch.
Document the corrected overload, including its optional error and null-handler removal behavior, and update the corresponding documentation known-failure entry. Add macOS regression coverage for both overloads, using task completion sources to avoid late callbacks touching disposed synchronization objects.
Fixes #26789
🤖 Pull request created by Copilot