From a4727252094d903b6ccbe455bf5f0b9e885d8114 Mon Sep 17 00:00:00 2001 From: Andrzej Martyna Date: Tue, 30 Dec 2025 09:06:57 +0100 Subject: [PATCH] Resolved: Thread-safety concern: _browserWindowsTracked modified without synchronization. --- .../Tabs/README.md | 3 +- .../Tabs/TabsTracker.cs | 38 +++++++++++++++---- .../Tabs/TabsWalker.cs | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md b/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md index fa41cf10e..fb561d7ed 100644 --- a/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md +++ b/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/README.md @@ -23,7 +23,8 @@ Next, each time the bookmark is triggered again, it just switches to the existin The extension bases on RuntimeId of AutomationElement from Microsoft UI Automation. This is a weak spot as browsers are used to regenerate internal structures and even reuse RuntimeId. -It **rarely** happens that a bookmark activates a wrong tab. +Therefore it happens that a bookmark activates a wrong tab. +The most common case is while user opens several tabs one after the other quickly. Still **"just take me to THIS place in milliseconds"** works almost all of the time so it brings so much value that it is worthwhile to accept the fact it fails sometimes. The quickest workaround is: diff --git a/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs b/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs index c0e3fd97b..588bc44c5 100644 --- a/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs +++ b/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsTracker.cs @@ -84,7 +84,12 @@ public class TabsTracker : IDisposable public void Dispose() { - var windowsToUnsubscribe = _browserWindowsTracked.ToList(); + List windowsToUnsubscribe; + lock (_sync) + { + windowsToUnsubscribe = _browserWindowsTracked.ToList(); + } + foreach (var wnd in windowsToUnsubscribe) { UnsubscribeStructureChangedForWindow(wnd); @@ -210,14 +215,22 @@ public class TabsTracker : IDisposable // break; case StructureChangeType.ChildRemoved: case StructureChangeType.ChildrenBulkRemoved: - foreach (var window in _browserWindowsTracked) + AutomationElement foundWindow = null; + lock (_sync) { - var windowRuntimeId = TabsCache.RuntimeIdToKey(window); - if (windowRuntimeId != null && eventRuntimeId.StartsWith(windowRuntimeId)) + foreach (var window in _browserWindowsTracked) { - _walker.RescanTabsForContainer(window); + var windowRuntimeId = TabsCache.RuntimeIdToKey(window); + if (windowRuntimeId != null && eventRuntimeId.StartsWith(windowRuntimeId)) + { + foundWindow = window; + } } } + if (foundWindow != null) + { + _walker.RescanTabsForContainer(foundWindow); + } break; } } @@ -232,11 +245,16 @@ public class TabsTracker : IDisposable private void UnsubscribeStructureChangedForWindow(AutomationElement wnd) { - if (_browserWindowsTracked.Contains(wnd)) + bool contains = false; + lock (_sync) + { + contains = _browserWindowsTracked.Contains(wnd); + _browserWindowsTracked.Remove(wnd); + } + if (contains) { Context.API.LogDebug(ClassName, "Unsubscribe window from StructureChanged events"); Automation.RemoveStructureChangedEventHandler(wnd, OnStructureChanged); - _browserWindowsTracked.Remove(wnd); _walker?.RemoveAllTabs(wnd); } } @@ -254,6 +272,10 @@ public class TabsTracker : IDisposable Automation.AddStructureChangedEventHandler(rootElement, TreeScope.Subtree, OnStructureChanged); Automation.AddAutomationEventHandler(WindowPattern.WindowClosedEvent, rootElement, TreeScope.Subtree, OnWindowClosed); - _browserWindowsTracked.Add(rootElement); + + lock (_sync) + { + _browserWindowsTracked.Add(rootElement); + } } } diff --git a/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs b/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs index c0e32a3f2..f73297eec 100644 --- a/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs +++ b/Plugins/Flow.Launcher.Plugin.BrowserBookmark/Tabs/TabsWalker.cs @@ -33,7 +33,7 @@ internal class TabsWalker continue; } - // on Chrome, there are kind of technical tabs that should be ignored + // There are kind of technical tabs that should be ignored var className = tab.Current.ClassName; if (className.Contains("bolt-tab", StringComparison.OrdinalIgnoreCase)) {