Updated wait operations to fix unreliable .editorconfig integration tests #34361
Conversation
| @@ -197,6 +197,12 @@ class C | |||
| MarkupTestFile.GetSpans(markup, out var text, out ImmutableArray<TextSpan> spans); | |||
| SetUpEditor(markup); | |||
| VisualStudio.WaitForApplicationIdle(CancellationToken.None); | |||
| VisualStudio.Workspace.WaitForAllAsyncOperations( | |||
| Helper.HangMitigatingTimeout, | |||
| FeatureAttribute.Workspace, | |||
There was a problem hiding this comment.
So it occurred to me this one is necessary, because there's the window between the file being opened and we haven't wired up to it. So it's not just a unknown reason...there's logic behind it.
|
|
||
| public void EndWaitForCodingConventionsChange(TimeSpan timeout) | ||
| { | ||
| var watcher = Interlocked.Exchange(ref _codingConventionsChangedWatcher, null); |
There was a problem hiding this comment.
Is the Interlocked.Exchange actually necessary for any threading reason or just a less annoying way to deal with the "copy to a local, then set the field to null"?
There was a problem hiding this comment.
The latter. None of the integration test out-of-proc/in-proc helpers are safe for concurrent use.
| { | ||
| var codingConventionsManager = GetComponentModelService<ICodingConventionsManager>(); | ||
| _codingConventionContext = codingConventionsManager.GetConventionContextAsync(filePath, CancellationToken.None).Result; | ||
| _codingConventionContext.CodingConventionsChangedAsync += (sender, e) => |
There was a problem hiding this comment.
We're not unsubscribing anywhere?
There was a problem hiding this comment.
Currently we are not. I can send a follow-up PR to the master branch to fix this after this one is merged.
This is an interim stability improvement while @jasonmalinowski continues to investigate asynchronous operations in this path.