-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix lock up that occurs when WinForms is executed on the pipeline thread #1149
Conversation
this.logger.LogTrace( | ||
"Attempting to get session details failed, most likely due to a running pipeline that is attempting to stop."); | ||
} | ||
// Get the session details and push the current |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosmetic change to fix misleading indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There's going to be a lot of this littered throughout the code base. We used to have a custom sync context that required us to be more selective about how we configured awaits. Now that we don't have that, we might want to:
- Set up the analyzer that warns when
ConfigureAwait(false)
isn't present - Configure every single await 🙂. It won't look great, but it'll save us a few out of no where deadlocks
Co-Authored-By: Patrick Meinecke <SeeminglyScience@users.noreply.github.com>
Here is an overview of what got changed by this pull request: Issues
======
+ Solved 3
- Added 1
See the complete overview on Codacy |
/// <summary> | ||
/// This is a bool but must be an int, since Interlocked.Exchange can't handle a bool | ||
/// </summary> | ||
private static int s_hasRunPsrlStaticCtor = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LMAO we finally got around to this, and I did for the OmniSharp library too. |
Fixes PowerShell/vscode-powershell#2352.
Dequeuing work to execute on the pipeline thread used to be async/await without using
ConfigureAwait(false)
. This meant that introducing a thread synchronisation context (like WinForms does) could break our thread architecture.Particularly in the WinForms case, the work dequeuing delegate would be marshalled back to the pipeline thread, which was blocked by the stopped debugger, so unable to execute.
Now we use
ConfigureAwait(false)
so that the lock delegate's execution is not influenced by the synchronisation context.