[System.Net.Http] Enable nullability in this framework.#24978
[System.Net.Http] Enable nullability in this framework.#24978rolfbjarne merged 3 commits intomainfrom
Conversation
And fix any build warnings/errors that came up. Contributes towards #17285.
There was a problem hiding this comment.
Pull request overview
Enables C# nullable reference types for the legacy CFNetworkHandler-based HTTP implementation and addresses resulting warnings/errors, as part of the broader effort to improve framework nullability.
Changes:
- Enabled
#nullableinCFNetworkHandlerandCFContentStream. - Added explicit null-handling for
HttpRequestMessage.RequestUri, proxy settings, stream events, and headers. - Annotated internal state to satisfy nullability (e.g.,
CookieContainer?,CFContentStream?,ExceptionDispatchInfo?,requiredfields).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/System.Net.Http/CFNetworkHandler.cs | Turns on nullability and updates request/stream handling to avoid nullable warnings. |
| src/System.Net.Http/CFContentStream.cs | Turns on nullability and updates buffering/error propagation logic accordingly. |
Comments suppressed due to low confidence (1)
src/System.Net.Http/CFNetworkHandler.cs:275
streamBucketsis a mutableDictionarythat’s accessed from multiple threads (SendAsync caller thread, CFRunLoop.Main event callbacks, and the cancellation callback). In this cancellation registration you readstreamBucketswithout locking, whileCloseStreammutates it under a lock and other code mutates it without a lock; concurrent reads/writes onDictionarycan throw or corrupt state. Use a consistent synchronization strategy (lock for all accesses, or switch to a thread-safe collection) so cancellation/event handling can’t race with add/remove.
bucket.CancellationTokenRegistration = cancellationToken.Register (() => {
if (!streamBuckets.TryGetValue (stream.Handle, out var bucket2))
return;
bucket2.Response.TrySetCanceled ();
CloseStream (stream);
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.
✅ [PR Build #5acf83f] 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 |
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #5acf83f] Build passed (Build packages) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #5acf83f] Build passed (Build macOS tests) ✅Pipeline on Agent |
🚀 [CI Build #5acf83f] 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 Linux Build VerificationPipeline on Agent |
And fix any build warnings/errors that came up.
Contributes towards #17285.