Refactor: Improve MudDebouncedInput, ThrottleDispatcher, DebounceDispatcher#12296
Conversation
- Redesigned DebounceDispatcher with proper thread-safety using SemaphoreSlim - Added IDisposable support to both dispatchers for proper resource cleanup - Implemented leading-edge debounce mode as constructor parameter - Updated DebounceDispatcher to properly dispose CancellationTokenSource instances - Exceptions from actions now properly propagate to callers - Added comprehensive test coverage for edge cases: * Cancellation scenarios * Disposal behavior * Thread-safety with concurrent calls * Zero/negative interval validation * Exception propagation * Leading vs trailing edge behavior - Redesigned ThrottleDispatcher with simpler, more robust implementation - Uses simple object lock instead of SemaphoreSlim for better performance - Returns same Task instance for calls within throttle interval - Properly handles task completion states - Added comprehensive test coverage for: * Multiple rapid calls returning same task * Exception handling and throttle reset * Disposal behavior * Cancellation token support * Thread-safety - Updated chart components (MudAxisChartBase, MudRadialChartBase, HeatMap) to use new leading-edge mode - All 4,163 unit tests pass (5 performance tests skipped) Co-authored-by: ScarletKuro <[email protected]>
- Clarified comment in DebounceDispatcher about re-throwing TaskCanceledException - Added documentation explaining synchronous cancellation in Dispose method - Added comment in ThrottleDispatcher explaining that action() is called synchronously Co-authored-by: ScarletKuro <[email protected]>
|
@copilot and are good in our case, yes usually it's a good call, but when dealing with Blazor sometimes the components might get disposed but something keep rolling for sometime, and I wouldn't want the component to propagate the exception to the browser / server (depending what rending mode is used), so a silent behavior is probably more preferred. This is why we actually silently ignore |
- Added proper disposal of ThrottleDispatcher and DebounceDispatcher in all components: * MudColorPicker: Dispose throttle dispatcher when interval changes * MudTabs: Dispose throttle dispatcher in DisposeAsync * MudAxisChartBase, MudRadialChartBase, HeatMap: Call Dispose instead of Cancel - Changed exception handling to be Blazor-friendly (silent instead of throwing): * ObjectDisposedException: Return silently instead of throwing * TaskCanceledException/OperationCanceledException: Return silently instead of throwing * This prevents exceptions from propagating to browser/server in Blazor components * User code exceptions still propagate normally - Updated all tests to reflect new silent behavior - All 4,163 tests pass Co-authored-by: ScarletKuro <[email protected]>
Addressed all concerns in commit 393067f: Disposal: ✅ All components now properly dispose dispatchers:
Exception handling: ✅ Changed to silent behavior for Blazor:
Performance: The lock/semaphore overhead is minimal:
Assert.ThrowsAsync: ✅ Removed - tests updated to reflect silent behavior Coverage: Working on it - the silent cancellation paths should improve coverage since tests now exercise those code paths instead of expecting exceptions. |
In DebounceDispatcher.Dispose is not covered. |
- Added tests to cover double-dispose scenarios: * DebounceDispatcher: Test calling Dispose() twice doesn't throw * ThrottleDispatcher: Test calling Dispose() twice doesn't throw * ThrottleDispatcher: Test dispose during lock acquisition is handled gracefully - Fixed DebounceDispatcher.Dispose to check _disposed before acquiring lock This prevents ObjectDisposedException when Dispose is called multiple times - Added TODO comment to replace object lock with System.Threading.Lock when targeting .NET 9+ - All 4,166 tests pass (added 3 new tests) - These changes improve code coverage for disposal paths Co-authored-by: ScarletKuro <[email protected]>
... Addressed in commit 6b81c94: Coverage improvements:
TODO for Lock class:
Technical details:
|
mudbot
bot
added
breaking change
|
@copilot can you make |
…terval API Changes to DebounceDispatcher: - Made _interval non-readonly to allow dynamic updates - Added UpdateInterval(int) and UpdateInterval(TimeSpan) methods - UpdateInterval is thread-safe and preserves pending debounce operations - Added comprehensive tests for UpdateInterval (8 new tests covering edge cases) Changes to MudDebouncedInput: - Replaced Timer-based implementation with DebounceDispatcher - Uses UpdateInterval() instead of recreating dispatcher when interval changes - Preserves pending debounce state across parameter changes - Fire-and-forget pattern with .CatchAndLog() maintains backward compatibility - Properly disposes DebounceDispatcher in DisposeAsyncCore Benefits: - Eliminates unnecessary object creation when interval changes - Preserves pending debounce state (important for complex scenarios) - Consistent debounce behavior across the codebase - Better resource management (no Timer allocations) - All 4,180 tests pass including all existing debounce tests Co-authored-by: ScarletKuro <[email protected]>
- Updated OnDebounceIntervalChanged to use DoubleEpsilonEqualityComparer.Default - Avoids unnecessary UpdateInterval calls when values are epsilon-equivalent - Added test to verify epsilon-equivalent values preserve debounce state - All 4,181 tests pass Co-authored-by: ScarletKuro <[email protected]>
- Changed UpdateInterval to UpdateIntervalAsync (async only, no sync version) - Fixed async void to async Task in OnDebounceIntervalChangedAsync - Changed .CatchAndLog() to _ = for fire-and-forget pattern - Updated all tests to properly await UpdateIntervalAsync calls - Updated Assert.Throws to Assert.ThrowsAsync for async methods - All 4,181 tests pass Co-authored-by: ScarletKuro <[email protected]>
- Use TimeSpan.FromMilliseconds instead of casting to int in MudDebouncedInput - This preserves fractional millisecond precision - _interval field is already thread-safe (reads/writes protected by SemaphoreSlim lock) - All tests pass Co-authored-by: ScarletKuro <[email protected]>
Revisit and Improve ThrottleDispatcher and DebounceDispatcher - COMPLETE
Successfully redesigned both dispatchers and migrated MudDebouncedInput to use the new DebounceDispatcher.
Final Implementation
DebounceDispatcher:
ThrottleDispatcher:
MudDebouncedInput:
Bug Fixes:
All 4,183 tests pass (2 new tests added)
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.