From e783c6213be69436a4df044fb5ddc1711e5b2cb7 Mon Sep 17 00:00:00 2001 From: Ilia Burakov Date: Sun, 23 Nov 2025 19:42:44 -0500 Subject: [PATCH 1/3] Ensures task completion before resource disposal Waits for mouse hook and event processing tasks to complete before disposing resources during shutdown. This prevents potential resource contention and ensures a clean shutdown. Adds logging for task completion and timeout scenarios. --- WinSyncScroll/ViewModels/MainViewModel.cs | 63 +++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/WinSyncScroll/ViewModels/MainViewModel.cs b/WinSyncScroll/ViewModels/MainViewModel.cs index 0ca4fab..1dbbabc 100644 --- a/WinSyncScroll/ViewModels/MainViewModel.cs +++ b/WinSyncScroll/ViewModels/MainViewModel.cs @@ -557,8 +557,71 @@ public void Dispose() _logger.LogError(e, "Error cancelling the cancellation token source"); } + // Wait for tasks to complete before disposing resources + // Using Task.Wait in Dispose is acceptable as this is a shutdown scenario + // and we need to ensure tasks complete before disposing underlying resources +#pragma warning disable VSTHRD002 // Synchronously waiting is acceptable in Dispose method + try + { + if (_updateMouseHookRectsLoopTask is not null + && !_updateMouseHookRectsLoopTask.IsCompleted + && !_updateMouseHookRectsLoopTask.Wait(TimeSpan.FromSeconds(5))) + { + _logger.LogWarning("Update mouse hook rects loop task did not complete within timeout"); + } + } + catch (AggregateException ae) + { + ae.Handle(ex => + { + if (ex is OperationCanceledException) + { + _logger.LogDebug("Update mouse hook rects loop was cancelled"); + return true; + } + + _logger.LogError(ex, "Error waiting for update mouse hook rects loop task"); + return true; + }); + } + catch (Exception e) + { + _logger.LogError(e, "Error disposing update mouse hook rects loop task"); + } + + try + { + if (_mouseEventProcessingLoopTask is not null + && !_mouseEventProcessingLoopTask.IsCompleted + && !_mouseEventProcessingLoopTask.Wait(TimeSpan.FromSeconds(5))) + { + _logger.LogWarning("Mouse event processing loop task did not complete within timeout"); + } + } + catch (AggregateException ae) + { + ae.Handle(ex => + { + if (ex is OperationCanceledException) + { + _logger.LogDebug("Mouse event processing loop was cancelled"); + return true; + } + + _logger.LogError(ex, "Error waiting for mouse event processing loop task"); + return true; + }); + } + catch (Exception e) + { + _logger.LogError(e, "Error disposing mouse event processing loop task"); + } +#pragma warning restore VSTHRD002 + + // Dispose tasks after they have completed _updateMouseHookRectsLoopTask?.Dispose(); _mouseEventProcessingLoopTask?.Dispose(); + _mouseHook.Dispose(); _cancellationTokenSource.Dispose(); } From 744e1b80bf8aa182654f84ddb2bfbfe33c92b8f4 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 09:06:47 +0800 Subject: [PATCH 2/3] Refactor: Extract duplicated task waiting logic into helper method (#21) * Initial plan * Extract duplicated task waiting logic into WaitForTaskCompletion helper method Co-authored-by: magicxor <8275793+magicxor@users.noreply.github.com> * Update WinSyncScroll/ViewModels/MainViewModel.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magicxor <8275793+magicxor@users.noreply.github.com> Co-authored-by: Ilia Burakov Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- WinSyncScroll/ViewModels/MainViewModel.cs | 66 ++++++++--------------- 1 file changed, 22 insertions(+), 44 deletions(-) diff --git a/WinSyncScroll/ViewModels/MainViewModel.cs b/WinSyncScroll/ViewModels/MainViewModel.cs index 1dbbabc..a36b5c8 100644 --- a/WinSyncScroll/ViewModels/MainViewModel.cs +++ b/WinSyncScroll/ViewModels/MainViewModel.cs @@ -545,29 +545,16 @@ public void HandleWindowClosing() } } - public void Dispose() + private void WaitForTaskCompletion(Task? task, string taskName, TimeSpan timeout) { - AppState = AppState.NotRunning; - try - { - _cancellationTokenSource.Cancel(); - } - catch (Exception e) - { - _logger.LogError(e, "Error cancelling the cancellation token source"); - } - - // Wait for tasks to complete before disposing resources - // Using Task.Wait in Dispose is acceptable as this is a shutdown scenario - // and we need to ensure tasks complete before disposing underlying resources -#pragma warning disable VSTHRD002 // Synchronously waiting is acceptable in Dispose method +#pragma warning disable VSTHRD002 // Synchronously waiting is acceptable in shutdown scenarios try { - if (_updateMouseHookRectsLoopTask is not null - && !_updateMouseHookRectsLoopTask.IsCompleted - && !_updateMouseHookRectsLoopTask.Wait(TimeSpan.FromSeconds(5))) + if (task is not null + && !task.IsCompleted + && !task.Wait(timeout)) { - _logger.LogWarning("Update mouse hook rects loop task did not complete within timeout"); + _logger.LogWarning("{TaskName} did not complete within timeout", taskName); } } catch (AggregateException ae) @@ -576,47 +563,38 @@ public void Dispose() { if (ex is OperationCanceledException) { - _logger.LogDebug("Update mouse hook rects loop was cancelled"); + _logger.LogDebug("{TaskName} was cancelled", taskName); return true; } - _logger.LogError(ex, "Error waiting for update mouse hook rects loop task"); + _logger.LogError(ex, "Error waiting for {TaskName} to complete", taskName); return true; }); } catch (Exception e) { - _logger.LogError(e, "Error disposing update mouse hook rects loop task"); + _logger.LogError(e, "Error waiting for {TaskName} to complete", taskName); } +#pragma warning restore VSTHRD002 + } + public void Dispose() + { + AppState = AppState.NotRunning; try { - if (_mouseEventProcessingLoopTask is not null - && !_mouseEventProcessingLoopTask.IsCompleted - && !_mouseEventProcessingLoopTask.Wait(TimeSpan.FromSeconds(5))) - { - _logger.LogWarning("Mouse event processing loop task did not complete within timeout"); - } - } - catch (AggregateException ae) - { - ae.Handle(ex => - { - if (ex is OperationCanceledException) - { - _logger.LogDebug("Mouse event processing loop was cancelled"); - return true; - } - - _logger.LogError(ex, "Error waiting for mouse event processing loop task"); - return true; - }); + _cancellationTokenSource.Cancel(); } catch (Exception e) { - _logger.LogError(e, "Error disposing mouse event processing loop task"); + _logger.LogError(e, "Error cancelling the cancellation token source"); } -#pragma warning restore VSTHRD002 + + // Wait for tasks to complete before disposing resources + // Using Task.Wait in Dispose is acceptable as this is a shutdown scenario + // and we need to ensure tasks complete before disposing underlying resources + WaitForTaskCompletion(_updateMouseHookRectsLoopTask, "Update mouse hook rects loop task", TimeSpan.FromSeconds(5)); + WaitForTaskCompletion(_mouseEventProcessingLoopTask, "Mouse event processing loop task", TimeSpan.FromSeconds(5)); // Dispose tasks after they have completed _updateMouseHookRectsLoopTask?.Dispose(); From bdbcdc498b75a951671da88c256e032e68742142 Mon Sep 17 00:00:00 2001 From: Ilia Burakov Date: Mon, 24 Nov 2025 09:11:26 +0800 Subject: [PATCH 3/3] Update WinSyncScroll/ViewModels/MainViewModel.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- WinSyncScroll/ViewModels/MainViewModel.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/WinSyncScroll/ViewModels/MainViewModel.cs b/WinSyncScroll/ViewModels/MainViewModel.cs index a36b5c8..63fcc9d 100644 --- a/WinSyncScroll/ViewModels/MainViewModel.cs +++ b/WinSyncScroll/ViewModels/MainViewModel.cs @@ -551,7 +551,6 @@ private void WaitForTaskCompletion(Task? task, string taskName, TimeSpan timeout try { if (task is not null - && !task.IsCompleted && !task.Wait(timeout)) { _logger.LogWarning("{TaskName} did not complete within timeout", taskName);