Repository navigation
fix(client): propagate CancellationToken through safe-mode setup (#123) - #124
Merged
Merged
Conversation
…q2GraphQL#123) Safe mode's one-time init (SemaphoreSlim wait + schema introspection) ran without the caller's token, so cancelling ExecuteAsync/ExecuteWithResultAsync could still block on the init lock and the introspection query. - InitQueryAsync: _lock.WaitAsync(ct) and GetSchemaForSafeModeAsync(ct) - GraphClient.GetSchemaForSafeModeAsync(ct): token flows into the introspection request; a cancelled fetch commits nothing to IMemoryCache, so the next call retries. If the fetch is ever made single-flight, it must not run on one caller's token (documented). - New token overloads for GetRequestAsync/GetRequestAsJsonAsync/ GetSchemaForSafeModeAsync; parameterless forms kept (binary compatible). - Tests: cancel during lock wait, during introspection, pre-cancelled token, cancelled fetch not cached / init retryable, waiter cancellation does not disturb the lock holder. Mutation-checked: dropping the token fails them.
Contributor
Author
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.
Fixes #123
Root cause
With
UseSafeModeon, the first query on aGraphQuery/GraphCursorQueryruns a one-time setup (InitQueryAsync): wait on aSemaphoreSlim, then fetch the schema viaGraphClient.GetSchemaForSafeModeAsync()unless it is already inIMemoryCache. Neither step took the caller'sCancellationToken, so cancellingExecuteAsync/ExecuteWithResultAsync(directly or through the cursor pager) could still block indefinitely on the setup lock and on the introspection query.Changes
InitQueryAsync(CancellationToken):_lock.WaitAsync(ct)(acquire outside thetry, so a cancelled wait cannot release a lock it never took) andGetSchemaForSafeModeAsync(ct).intializedis only set after the whole setup succeeded, so a cancelled init is simply retried by the next call.GraphClient.GetSchemaForSafeModeAsync(CancellationToken): the token flows into the introspection request. Safety of passing a per-caller token into theIMemoryCache.GetOrCreateAsyncfactory is documented on the method:GetOrCreateAsyncis not single-flight (each cache miss runs its own factory) and a factory that throws - including on cancellation - commits nothing, so a cancelled fetch is never cached and other callers are unaffected.GraphBaseExecute.GetRequestAsync(CancellationToken),GetRequestAsJsonAsync(CancellationToken)andGraphClient.GetSchemaForSafeModeAsync(CancellationToken); the existing parameterless methods are kept, so this is binary compatible. (Per .NET library guidance, adding a parameter to an existing method is a breaking change, hence overloads.)ExecuteBaseAsync/ExecuteBaseWithResultAsyncpass their token toGetRequestAsync(ct); the cursor pager already forwards its token there.Not changed
Linq2GraphQL.Client.Subscriptions:StartAsync()/StartWithErrorsAsync()accept noCancellationTokentoday, so the parameterlessGetRequestAsync()call remains. AStartAsync(CancellationToken)overload would be a natural follow-up.Testing
GraphBaseExecuteTests(in-process handlers, no live server): cancel while waiting for the setup lock (holder is not disturbed), cancel during introspection, pre-cancelled token sends no request, cancelled schema fetch is not cached and init is retried successfully, plus overload-compat coverage of the parameterless forms.HttpClient.Timeout = Timeout.InfiniteTimeSpan(the 100 s default timeout produces aTaskCanceledExceptionthat would satisfy anOperationCanceledExceptionassertion) and every bounded wait goes throughTask.WaitAsync(10 s).ExecuteBase*call) fails at least two of the new tests.dotnet build Linq2GraphQL.CI.slnf: 0 errors / 0 warnings.dotnet test Linq2GraphQL.CI.slnf: 454/454 passed.dotnet format --verify-no-changeson the touched files: clean.