Thread Safety or: How I Learned to Stop Polling Bools and Love the WaitHandles

[dodexahedron]

We should discuss and address thread safety issues at some point. I've identified a few and notated some as I've been working on tests.

Here's a placeholder to get the discussion going.

[tznind]

LockSelection is not a thread lock. It is a condition that prevents modifying the currently 'selected' views in the Editor.

So threads/operations do not 'wait' to modify the selection, they simply do not modify it at all but otherwise continue to execute normally.

SelectionManager controls what views are 'selected'. I use single quotes because it is seperate from what has input Focus. When you drag a selection box over some views they become selected:

This uses the SetSelection(params Design[] designs) method.

If an Operation (or anyone else) wants to change the selection they call this method. Typically while executing operations we prevent this selection from changing. But there are situations where we want to change the selection mid execution e.g. adding a view (select the new view) or delete (clear view selection). In this case the operation will use ForceSetSelection(params Design[] designs).

There may be a way to simplify this but its not related to classical thread/wait locking.

[dodexahedron]

Well, even if not intended for threads by design, in practice and at run-time, that's still a synchronization mechanism, there to solve the problem of needing exclusive access to a critical section or object, and it does address that, on the surface, but is vulnerable to race conditions and dirty reads.

It's one of the components of why most tests can't run in parallel, even when they are doing different things on different types. If they involve any of the singletons, you get a mixed bag of different failures with different exceptions from different tests (or sometimes they even pass!).

It's of course just one and the work to make things thread-safe isn't a small task, and not one that I'm suggesting we need to handle with any urgency, right now - just something we should keep in mind for later and as work is done in the present.

[dodexahedron]

Just a follow-up thought: Attempting to run the unit tests aggressively in parallel is kind of a nice synthetic way of at least usually forcing an excessive amount of concurrency issues. But, if/when/as the issues are addressed, that incidental fact will become less and less effective, and it would be wise to, as an even longer-term work item, have tests explicitly intended to test concurrency.

While that's of course way more important in TG itself, more than it is in TGD, it's still important in TGD, because, even if we aren't explicitly firing off threads, concurrency issues still rear their ugly heads because of reentrancy and other things, especially with events and things like IDisposable.