From 0a147a8100c992ab6e29fa0f0b6a66650928a53e Mon Sep 17 00:00:00 2001 From: leishifu666 <191225580+leishifu666@users.noreply.github.com> Date: Mon, 19 Oct 2026 03:11:34 +0800 Subject: [PATCH] Fix global gesture hook restart race --- .../Services/Triggers/TriggerService.cs | 49 +++++- .../TriggerServiceLifecycleTests.cs | 151 ++++++++++++++++++ 2 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 tests/Filee.App.Tests/TriggerServiceLifecycleTests.cs diff --git a/src/Filee.App/Services/Triggers/TriggerService.cs b/src/Filee.App/Services/Triggers/TriggerService.cs index b438ce3b..e1226487 100644 --- a/src/Filee.App/Services/Triggers/TriggerService.cs +++ b/src/Filee.App/Services/Triggers/TriggerService.cs @@ -20,15 +20,23 @@ public sealed class TriggerService : IDisposable private readonly GestureDetector _detector = new(); private readonly string _ownProcess = Process.GetCurrentProcess().ProcessName; private EventLoopGlobalHook? _hook; + private Thread? _hookThread; + private readonly Func _createHook; private HashSet _excluded = new(StringComparer.OrdinalIgnoreCase); /// FILEE_TRACE_GESTURES=1 logs each mouse press and scope decision, to find why a gesture doesn't fire. private static readonly bool Trace = Environment.GetEnvironmentVariable("FILEE_TRACE_GESTURES") == "1"; public TriggerService(IPlatformServices platform, ILogger log) + : this(platform, log, () => new EventLoopGlobalHook(useBackgroundThreadForEventLoop: true)) + { + } + + internal TriggerService(IPlatformServices platform, ILogger log, Func createHook) { _platform = platform; _log = log; + _createHook = createHook; _detector.SystemDragThreshold = platform.SystemDragThreshold; _detector.ScopeFilter = InScope; @@ -98,7 +106,7 @@ public void Start() try { // Background threads: a hook that fails to stop can never keep Filee running after it quits. - _hook = new EventLoopGlobalHook(useBackgroundThreadForEventLoop: true); + _hook = _createHook(); _hook.HookEnabled += (_, _) => { _log.LogInformation("Global input hook started"); NotifyStatus(); }; _hook.HookDisabled += (_, _) => { _log.LogInformation("Global input hook stopped"); NotifyStatus(); }; _hook.MousePressed += (_, e) => @@ -111,15 +119,26 @@ public void Start() _hook.MouseDragged += (_, e) => OnMove(e); _hook.MouseMoved += (_, e) => OnMove(e); _hook.KeyPressed += (_, e) => _detector.KeyDown(KeyName(e.Data.KeyCode), Map(e.RawEvent.Mask)); - _ = _hook.RunAsync(useBackgroundThread: true).ContinueWith(t => + // Own the actual Run thread. SharpHook's RunAsync task completes BEFORE its finally + // clears the native callback, so awaiting that task alone is not a safe restart barrier. + var hook = _hook; + _hookThread = new Thread(() => { - // e.g. macOS without Accessibility permission. Drop zone and context menu keep working. - if (t.Exception is not null) + try + { + hook.Run(); + } + catch (Exception ex) + { + ReportFailure(ex.GetBaseException(), "Global input hook failed; gestures are unavailable"); + } + finally { - ReportFailure(t.Exception.GetBaseException(), "Global input hook failed; gestures are unavailable"); NotifyStatus(); } - }, TaskScheduler.Default); + }) + { IsBackground = true, Name = "Filee global input" }; + _hookThread.Start(); } catch (Exception ex) { @@ -127,6 +146,7 @@ public void Start() ReportFailure(ex, "Could not start the global input hook"); _hook?.Dispose(); _hook = null; + _hookThread = null; NotifyStatus(); } } @@ -232,8 +252,25 @@ public void Dispose() { // Quitting goes on: the hook runs on background threads, which end with the process. _log.LogWarning(ex, "Could not stop the global input hook"); + // SharpHook marks the hook disposed before Stop can throw. Its finalizer must not + // dispose the already-completed event queue again; we still own and join its Run thread. + if (_hook is { IsDisposed: true } failedHook) + GC.SuppressFinalize(failedHook); + Error = ex.Message; + NotifyStatus(); + return; // Keep ownership: starting a second native hook would corrupt the first one. + } + // Dispose only requests native shutdown. Its Run thread must finish its finally block + // before another hook installs the process-wide callback, or the old thread clears it. + if (_hookThread is { IsAlive: true } thread && !thread.Join(TimeSpan.FromSeconds(5))) + { + Error = "The previous global input hook did not finish stopping. Retry after it exits."; + _log.LogWarning("Global input hook shutdown timed out; keeping the previous hook to prevent overlap"); + NotifyStatus(); + return; } _hook = null; + _hookThread = null; } } diff --git a/tests/Filee.App.Tests/TriggerServiceLifecycleTests.cs b/tests/Filee.App.Tests/TriggerServiceLifecycleTests.cs new file mode 100644 index 00000000..b7db9536 --- /dev/null +++ b/tests/Filee.App.Tests/TriggerServiceLifecycleTests.cs @@ -0,0 +1,151 @@ +// Verifies that retrying gestures waits for the previous native hook to release its shared callback. + +using Avalonia.Headless.XUnit; +using Filee.App.Services.Triggers; +using Filee.Core.Platform; +using Microsoft.Extensions.Logging.Abstractions; +using SharpHook; +using SharpHook.Data; +using SharpHook.Providers; + +namespace Filee.App.Tests; + +public sealed class TriggerServiceLifecycleTests +{ + [AvaloniaFact] + public async Task Retry_waits_for_old_native_cleanup_and_keeps_new_callback() + { + var provider = new DelayedProvider(); + var keys = 0; + using var service = Create(provider, () => Interlocked.Increment(ref keys)); + service.Start(); + Assert.True(SpinWait.SpinUntil(() => provider.Runs == 1, 2000)); + var cancellation = TestContext.Current.CancellationToken; + var stopping = Task.Factory.StartNew(service.Dispose, cancellation, TaskCreationOptions.LongRunning, TaskScheduler.Default); + try + { + Assert.True(provider.StopRequested.Wait(2000, cancellation)); + Assert.NotSame(stopping, await Task.WhenAny(stopping, Task.Delay(75, cancellation))); + } + finally + { + provider.AllowExit.Set(); + await stopping.WaitAsync(TimeSpan.FromSeconds(2), cancellation); + } + Assert.False(provider.HasCallback); + service.Start(); + Assert.True(SpinWait.SpinUntil(() => provider.Runs == 2, 2000)); + provider.PressKey(); + Assert.True(SpinWait.SpinUntil(() => Volatile.Read(ref keys) == 1, 2000)); + Assert.True(service.IsRunning); + Assert.True(provider.HasCallback); + } + + [AvaloniaFact] + public void Repeated_retries_keep_delivering_input() + { + var provider = new DelayedProvider(); + provider.AllowExit.Set(); + var keys = 0; + using var service = Create(provider, () => Interlocked.Increment(ref keys)); + for (var i = 1; i <= 30; i++) + { + service.Start(); + Assert.True(SpinWait.SpinUntil(() => provider.Runs == i, 2000)); + provider.PressKey(); + var expected = i; + Assert.True(SpinWait.SpinUntil(() => Volatile.Read(ref keys) == expected, 2000)); + service.Dispose(); + } + Assert.Equal(30, keys); + } + + [AvaloniaFact] + public void Failed_stop_does_not_start_a_second_hook() + { + var provider = new DelayedProvider { FailStop = true }; + provider.AllowExit.Set(); + using var service = Create(provider, () => { }); + service.Start(); + Assert.True(SpinWait.SpinUntil(() => provider.Runs == 1, 2000)); + try + { + service.Dispose(); + Assert.NotNull(service.Error); + service.Start(); + Assert.Equal(1, provider.Runs); + } + finally + { + provider.StopRequested.Set(); + service.Dispose(); + } + } + + private static TriggerService Create(DelayedProvider provider, Action key) => + new(new NullPlatformServicesForHooks(), NullLogger.Instance, () => + { + var hook = new EventLoopGlobalHook(provider, useBackgroundThreadForEventLoop: true); + hook.KeyPressed += (_, _) => key(); + return hook; + }); + + // Models libuiohook's process-wide callback and asynchronous native shutdown. No OS input is hooked. + private sealed class DelayedProvider : IGlobalHookProvider + { + private readonly object _gate = new(); + private DispatchProc? _callback; + private nint _data; + private int _runs; + public readonly ManualResetEventSlim StopRequested = new(); + public readonly ManualResetEventSlim AllowExit = new(); + public bool FailStop; + public bool KeyTypedEnabled { get; set; } + public int Runs => Volatile.Read(ref _runs); + public bool HasCallback { get { lock (_gate) return _callback is not null; } } + + public void SetDispatchProc(DispatchProc? callback, nint data) + { + lock (_gate) { _callback = callback; _data = data; } + } + public UioHookResult Run() + { + StopRequested.Reset(); + Interlocked.Increment(ref _runs); + StopRequested.Wait(); + AllowExit.Wait(); + return UioHookResult.Success; + } + public UioHookResult RunKeyboard() => Run(); + public UioHookResult RunMouse() => Run(); + public UioHookResult Stop() + { + if (FailStop) return UioHookResult.Failure; + StopRequested.Set(); + return UioHookResult.Success; + } + public void PressKey() + { + lock (_gate) + { + var e = new UioHookEvent { Type = EventType.KeyPressed, Keyboard = new KeyboardEventData { KeyCode = KeyCode.VcA } }; + _callback?.Invoke(ref e, _data); + } + } + } + + private sealed class NullPlatformServicesForHooks : IPlatformServices + { + public bool IsFileManagerAt(int x, int y) => true; + public string? ProcessNameAt(int x, int y) => "explorer"; + public IReadOnlyList GetFileManagerSelection() => []; + public int SystemDragThreshold => 4; + public bool PrefersReducedMotion => false; + public void SetStartWithSystem(bool enabled, string executablePath) { } + public void SetContextMenu(bool enabled, string executablePath, string label) { } + public ModernContextMenuState GetModernContextMenuState(string executablePath) => ModernContextMenuState.Unsupported; + public Task SetModernContextMenuAsync(bool enabled, string executablePath) => Task.FromResult(new ModernContextMenuResult(false)); + public void RevealInFileManager(string path) { } + public void RaiseTopmost(nint windowHandle) { } + } +}