Repository navigation
fix(core): complete disposed update feeds without faulted tasks - #11661
Open
KSemenenko wants to merge 4 commits into
Open
KSemenenko wants to merge 4 commits into
KSemenenko wants to merge 4 commits into
Conversation
Complete the terminal AsyncEnumerable element successfully instead of creating an ObjectDisposedException task which has no reader. Preserve pending and future enumeration completion, Current and publication refusal, idempotent disposal, and isolated reader cancellation. The new disposal resource test fails before on net8.0 and net10.0. Strict scoped builds pass on both targets, with 141 focused membership, manifest and update-feed cases per target. An isolated native check of 100 disposals produces 100 unobserved failures before and 0 after.
Contributor
Code coverage
Report-only conclusion: current-main baseline stale. The newest successful coverage run tested 9a0c764, not current main 625b2ba. Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
ProtoIntClass declares messages without services. Keep C# protobuf model generation while avoiding the unused gRPC C# plugin, which the current Grpc.Tools package supplies only as macOS x64.
Exercise slow readers through the terminal element and verify that disposed functional updates cannot invoke user callbacks. These controls protect feed completion and publication contracts alongside the disposal resource regression.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disposing an internal membership, manifest or directory update feed creates a faulted terminal task that no reader can reach. Shutdown itself succeeds, but later GC raises an unobserved
ObjectDisposedException, even when the feed has never been enumerated.Cause and resource impact
Readers reach the disposed element through the previous element's successfully completed next-link task.
MoveNextAsyncthen sees the disposed sentinel and returnsfalse, so the terminal element's own faulted_nexttask is never awaited.That path allocates an exception and fault-handling state and later performs unobserved-fault reporting/finalization work for a failure which is not part of the reader contract. The terminal element, completion source and task remain part of the representation after this change; exact allocated bytes, CPU savings and throughput gains have not been measured. The demonstrated cost is per helper disposal, rather than per successful request or enumeration step.
Correction and preserved behavior
Complete the terminal element's next link successfully with the terminal element itself. The disposed sentinel still ends enumeration, and the existing guards keep readers from following that link. The collectible self-reference does not keep the source alive.
The focused tests preserve pending and future reader completion, buffered values for slow readers, idempotent disposal,
Currentthrowing after disposal, post-disposal publication refusal for both direct and functional updates, and independent reader cancellation. Disposed functional updates do not execute user callbacks. The collectible self-reference does not keep the source alive.Evidence
The new resource regression fails before this change on both net8.0 and net10.0; the reader-completion and cancellation controls already pass before the change. Separate native checks using the released 10.4.2 package and original main DLL produce 100 unobserved notifications for 100 disposals. The corrected DLL produces zero with the same operation and GC sequence.
Afterward, all 143 focused membership, manifest and update-feed cases pass on each target, net8.0 and net10.0. The branch includes current main
625b2baf761fa3cbfc35d1125c6b0d3244fdd847. The canonical full-solution build passes with warnings treated as errors: zero warnings and zero errors. Whitespace verification also passes.Build validation also marks the message-only benchmark protobuf input with
GrpcServices="None", matching the serialization test project. Protobuf C# model generation still runs; the benchmark no longer invokes a gRPC service plugin for an input without services. The native ARM64 validation uses official protoc 33.5 through the supportedPROTOBUF_PROTOCsetting.Closes #11660
Microsoft Reviewers: Open in CodeFlow