Force the OneTimeTearDown to be executed on the current thread if STA

[EraserKing]

This aims to be a workaround for #3961

When both SingleThreaded and Parallizable attributes are attached to the test fixture, SingleThreadedAttribute.ApplyToContext may be executed later than expected (even after all test methods in the test fixtures are executed), so it leads to the following result:

1. The parent context still has IsSingleThreaded false, and all the test methods are executed on this context: (targetApartment and currentApartment are both Unknown, in WorkItem.Execute), so WorkItem.RunOnCurrentThread is always called.

2. Then, when the OneTimeTearDown of test fixture is executed, a new OneTimeTearDownWorkItem is created based on the current work item (see CompositeWorkItem.OnAllChildItemsCompleted), which inherits the parent ExecutionStrategy. At that time, since the Context.IsSingleThreaded may still haven't been set to true, the ExecutionStrategy will be Parallel.

3. At this time, not sure about the call stack, SingleThreadedAttribute.ApplyToContext is called and IsSingleThreaded set to true.

4. When the one time tear down work item is actually executed, though Context.IsSingleThreaded is now true, but the ParallelExecutionStrategy is already set in step 2, so it will still fall into the ParallelExecutionStrategy.Parallel branch and executed by a parallel queue, which may happen on another thread (see ParallelWorkItemDispatcher.Dispatch).

5. The PR adds a workaround, when it's going to execute the work item, double-check if Context.IsSingleThreaded is set to true, and if so, force it to run on the current thread.

The clean fix should be investiage why step 3 is delayed but that's beyond my ability. Thanks.

[dnfadmin]

All CLA requirements met.

[stevenaw]

NOTE to self or other reviewers. This may also fix:

• SingleThreadedAttribute ignored on OneTimeTearDown with ParallelScope set to All or Fixtures #2610
• ParallelScope.Fixtures - (Sometimes) Different Threads Perform OneTimeSetUp & TearDown #3426

[OsirisTerje]

@stevenaw You looked at this earlier. It doesn't come with any tests, but do (or did) you see any possible side-effects of this? If not, any reason for not merging it in?

[stevenaw]

I hadn't had the chance to pull and run the code, but the changes "look" safe to me and appear to be backwards compatible too. I'd be fine if it were merged in.

I think I'd mostly avoided any explicit action on it earlier out of a desire to dig into if there was also any relation to the changes in the underlying bug fixed by #4099 , but that investigation doesn't warrant holding this up.

Not sure if we also prefer Rob review since he self-assigned, but I can approve from my own involvement in this. I understand he's a little busy right now. Otherwise, feel free to merge this in @OsirisTerje 🙂