Resolved: Thread-safety concern: _browserWindowsTracked modified without synchronization.

This commit is contained in:
Andrzej Martyna 2025-12-30 09:06:57 +01:00
parent cdb2d9e6dc
commit a472725209
3 changed files with 33 additions and 10 deletions

View file

@ -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:

View file

@ -84,7 +84,12 @@ public class TabsTracker : IDisposable
public void Dispose()
{
var windowsToUnsubscribe = _browserWindowsTracked.ToList();
List<AutomationElement> 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);
}
}
}

View file

@ -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))
{