Fix CancellationTokenSource disposal issues in multiple components (#12249)#12255
Open
msynk wants to merge 4 commits intobitfoundation:developfrom
Open
Fix CancellationTokenSource disposal issues in multiple components (#12249)#12255msynk wants to merge 4 commits intobitfoundation:developfrom
msynk wants to merge 4 commits intobitfoundation:developfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe pull request addresses cancellation token source disposal issues across multiple Blazor UI components. Changes add explicit cancellation signaling before disposal and introduce disposal guards to prevent further execution after component disposal, ensuring clean teardown without exceptions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 task
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a set of disposal-related race conditions across Bit.BlazorUI input components where CancellationTokenSource instances could be disposed while still in use (or not cancelled before disposal), leading to ObjectDisposedException and runaway “continuous change” loops after component disposal.
Changes:
- Add
IsDisposedshort-circuits in pointer-down handlers and continuous-change loops to stop processing after user callbacks dispose the component. - Ensure CTS instances are cancelled before being disposed in
DisposeAsyncacross multiple components. - Add disposal cleanup for tooltip delay CTSs and cancel pending data-load CTS in the DataGrid.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Tooltip/BitTooltip.razor.cs | Cancels/disposes tooltip show/hide delay CTSs during disposal. |
| src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs | Cancels global CTS before disposal to stop pending operations. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cs | Adds IsDisposed guards and cancels CTS on dispose to stop continuous time changes safely. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cs | Cancels CTS on dispose to stop pending suggestion-provider calls. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/NumberField/BitNumberField.razor.cs | Prevents CTS usage after disposal and stops continuous increment/decrement loop cleanly. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cs | Adds disposal guards and CTS cancellation to stop continuous time spinner loop. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cs | Adds disposal guards and CTS cancellation to stop continuous time spinner loop. |
| src/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cs | Adds disposal guard in loop entry/reset and cancels CTS on dispose (but still has a remaining race; see comments). |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs | Cancels global CTS before disposal to stop pending operations. |
| src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs | Cancels/disposes pending data-load CTS during disposal. |
Comments suppressed due to low confidence (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cs:1081
ContinuousChangeTimecallsChangeTime(...)and then immediately callsStateHasChanged()/recurses without re-checkingIsDisposed. In this component,ChangeTimeupdatesCurrentValue, which kicks offSetCurrentValueAsync(fire-and-forget) that can invoke user callbacks and dispose the component before the subsequentStateHasChanged()/recursion runs. Add anIsDisposedcheck afterChangeTime(and similarly inHandleOnPointerDownright afterChangeTimeand beforeResetCts/starting the background loop) so the loop exits cleanly when a value-change callback removes the component.
private async Task ContinuousChangeTime(bool isNext, bool isHour, CancellationTokenSource cts)
{
if (cts.IsCancellationRequested || IsDisposed) return;
ChangeTime(isNext, isHour);
StateHasChanged();
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Tooltip/BitTooltip.razor.cs
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cs (1)
656-664:⚠️ Potential issue | 🟠 MajorAdd cancellation and disposal guards to
SearchItems()provider call.Line 390 fire-and-forgets
SearchItems(), and theSuggestItemsProviderawait at line 532 receives a cancellation token that can be cancelled from both DisposeAsync (line 662) and subsequent keystroke events. If a provider properly honors the token by throwingOperationCanceledException, the unobserved task faults. Additionally, state is modified after the await without checking if the component was disposed or if the operation was cancelled. Wrap the provider call in try-catch forOperationCanceledExceptionand add a stale-guard check before continuing into the callout path.Suggested fix
else if (SuggestItemsProvider is not null) { _cancellationTokenSource?.Cancel(); _cancellationTokenSource?.Dispose(); - _cancellationTokenSource = new(); - _viewSuggestedItems = [.. (await SuggestItemsProvider(new(CurrentValue, MaxSuggestCount, _cancellationTokenSource.Token))).Take(MaxSuggestCount)]; + var cts = _cancellationTokenSource = new(); + + try + { + var items = await SuggestItemsProvider(new(CurrentValue, MaxSuggestCount, cts.Token)); + if (IsDisposed || cts.IsCancellationRequested || _cancellationTokenSource != cts) return; + + _viewSuggestedItems = [.. items.Take(MaxSuggestCount)]; + } + catch (OperationCanceledException) when (cts.IsCancellationRequested) + { + return; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cs` around lines 656 - 664, SearchItems() currently fire-and-forgets a call to the SuggestItemsProvider which can be cancelled via DisposeAsync (_cancellationTokenSource) or by new keystrokes; wrap the provider invocation in a try/catch that catches OperationCanceledException and returns silently (to avoid unobserved task faults), and before applying any state changes after the await check both the provided CancellationToken (token.IsCancellationRequested) and the component IsDisposed flag to guard against stale results; update the code in SearchItems() (and any helper that awaits SuggestItemsProvider) to use the same token from _cancellationTokenSource, catch OCE, and only call StateHasChanged/update callout UI if not canceled and not disposed.src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs (1)
500-505:⚠️ Potential issue | 🟠 MajorHandle
OperationCanceledExceptionfrom the items provider in the non-virtualized refresh path.Line 345 awaits
ResolveItemsRequestAsync(request)without catchingOperationCanceledException, while Line 327 cancels the same CTS and DisposeAsync (lines 504-505) also cancels it. AnItemsProviderthat propagatesOperationCanceledExceptionwill throw unhandled instead of cleanly aborting, even though the check at line 346 suggests cancellation was anticipated. The same gap exists inProvideVirtualizedItemsat line 381.Wrap the provider call in a try/catch for
OperationCanceledExceptionwhenthisLoadCts.IsCancellationRequestedis true to treat cancellation as a normal exit path:Suggested fix
- var result = await ResolveItemsRequestAsync(request); + BitDataGridItemsProviderResult<TGridItem> result; + try + { + result = await ResolveItemsRequestAsync(request); + } + catch (OperationCanceledException) when (thisLoadCts.IsCancellationRequested) + { + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs` around lines 500 - 505, Resolve unhandled OperationCanceledException from the items provider by wrapping calls to ResolveItemsRequestAsync(request) (non-virtualized refresh path) and the analogous call in ProvideVirtualizedItems in a try/catch that catches OperationCanceledException; inside the catch, check thisLoadCts.IsCancellationRequested (or the relevant CancellationToken) and silently return/exit the method to treat cancellation as normal, and only rethrow if the token was not cancelled; also ensure DisposeAsync cancels and disposes _pendingDataLoadCancellationTokenSource as it already does so that cancellations can occur safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs`:
- Around line 204-208: After cancelling and disposing _globalCts you must
prevent LoadMoreItems from unconditionally calling StateHasChanged during
teardown; update LoadMoreItems to check the cancellation state (e.g. inspect
_globalCts?.IsCancellationRequested or the active CancellationToken) and
early-return when cancellation has been requested or the token/source is null,
so StateHasChanged is not invoked after _globalCts is cancelled/disposed;
reference _globalCts, LoadMoreItems, and StateHasChanged to locate and apply the
guard.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs`:
- Around line 278-282: PerformLoadMore() can still call the unconditional
StateHasChanged after _globalCts is Cancel()ed/Dispose()d; guard the trailing
render by checking the cancellation state before invoking StateHasChanged.
Update PerformLoadMore() to capture/inspect the same CTS (or its Token) used by
the teardown and only call StateHasChanged when the CTS is non-null and not
IsCancellationRequested (or when an _isDisposed flag is false); ensure the
cancellation branch in Dispose/teardown sets the same signal so no render runs
after _globalCts.Cancel()/Dispose().
---
Outside diff comments:
In `@src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cs`:
- Around line 500-505: Resolve unhandled OperationCanceledException from the
items provider by wrapping calls to ResolveItemsRequestAsync(request)
(non-virtualized refresh path) and the analogous call in ProvideVirtualizedItems
in a try/catch that catches OperationCanceledException; inside the catch, check
thisLoadCts.IsCancellationRequested (or the relevant CancellationToken) and
silently return/exit the method to treat cancellation as normal, and only
rethrow if the token was not cancelled; also ensure DisposeAsync cancels and
disposes _pendingDataLoadCancellationTokenSource as it already does so that
cancellations can occur safely.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cs`:
- Around line 656-664: SearchItems() currently fire-and-forgets a call to the
SuggestItemsProvider which can be cancelled via DisposeAsync
(_cancellationTokenSource) or by new keystrokes; wrap the provider invocation in
a try/catch that catches OperationCanceledException and returns silently (to
avoid unobserved task faults), and before applying any state changes after the
await check both the provided CancellationToken (token.IsCancellationRequested)
and the component IsDisposed flag to guard against stale results; update the
code in SearchItems() (and any helper that awaits SuggestItemsProvider) to use
the same token from _cancellationTokenSource, catch OCE, and only call
StateHasChanged/update callout UI if not canceled and not disposed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a0e270b1-59ff-4592-a723-f7b49ddc1ed4
📒 Files selected for processing (10)
src/BlazorUI/Bit.BlazorUI.Extras/Components/DataGrid/BitDataGrid.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Calendar/BitCalendar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DatePicker/BitDatePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/DateRangePicker/BitDateRangePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/NumberField/BitNumberField.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/SearchBox/BitSearchBox.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/TimePicker/BitTimePicker.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Tooltip/BitTooltip.razor.cs
src/BlazorUI/Bit.BlazorUI.Extras/Components/InfiniteScrolling/BitInfiniteScrolling.razor.cs
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs
Show resolved
Hide resolved
Cyrus-Sushiant
approved these changes
Apr 13, 2026
src/BlazorUI/Bit.BlazorUI/Components/Lists/BasicList/BitBasicList.razor.cs
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Inputs/NumberField/BitNumberField.razor.cs
Show resolved
Hide resolved
Contributor
|
Thank you for the fix! I looked at the changes and everything seems alright. I tested the reported issues and all is working well now |
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.
closes #12249
Summary by CodeRabbit