Potential race with BaseToolWindow<T>.Initialize about community.visualstudio.toolkit HOT 5 CLOSED

is the BaseToolWindow<T>.Initialize API meant to be thread safe?

🤔 The tool windows are registered against the ToolkitPackage and inside the package they're not stored in static fields or anything like that, so no chance of two different packages registering tool windows into the same collection. Under normal circumstances, I would expect that the Initialize methods of two different tool windows would be called sequentially, and that would be done before switching to the main thread (my understanding is that you should try to do as much work as possible before switching to the main thread). For example:

MyFirstToolWindow.Initialize();
MySecondToolWindow.Initialize();

await JoinableTaskFactory.SwitchToMainThreadAsync();

So with that in mind, I would say that no, it's not meant to be thread-safe.

from community.visualstudio.toolkit.

@bluetarpmedia @reduckted what if it used a ConcurrentList or similar? Would that stop a potential race condition?

from community.visualstudio.toolkit.

Yeah, a lock around any usage of _toolWindowProviders would solve it, or another option would be to enforce that the methods that read/write that list all run on the same thread (e.g. the UI thread, by calling ThreadHelper.ThrowIfNotOnUIThread at the start of AddToolWindow, GetToolWindowProvider, or GetAsyncToolWindowFactory).

But I'm not sure whether this is a problem that needs to be solved because to get a race condition I think the user is going to need to do something a bit odd.

It reminds me of the saying about protecting against Murphy (accidental misuse) vs Machiavelli (deliberate abuse). I think, yes, it should try and protect against accidental misuse of the API, but it's not worth trying to protect against deliberately silly code that's attempting to break it. So that poses the question: is the BaseToolWindow<T>.Initialize API meant to be thread safe?

from community.visualstudio.toolkit.

@bluetarpmedia

protecting against Murphy (accidental misuse) vs Machiavelli (deliberate abuse)

I hadn't heard that expression. I like it!

from community.visualstudio.toolkit.

I'm closing this as the intention is for Initialize to not be thread-safe.

from community.visualstudio.toolkit.