Skip to content
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

Avoid async deadlock in InteractiveHost... #7775

Merged
merged 2 commits into from
Jan 5, 2016

Conversation

KevinH-MS
Copy link
Contributor

The scheduler returned by TaskScheduler.FromCurrentSynchronizationContext
does not allow concurrency (meaning all Tasks within a submission were being
inlined). This would nearly always result in a deadlock if you kicked off a
child Task and attempted to continue within the same submission.

We can instead use the Winforms Dispatcher, which will invoke the parent
Task on the "UI" thread but allow child Tasks to use the ThreadPool.

Fixes #7280

The scheduler returned by TaskScheduler.FromCurrentSynchronizationContext
does not allow concurrency (meaning all Tasks within a submission were being
inlined).  This would nearly always result in a deadlock if you kicked off a
child Task and attempted to continue within the same submission.

We can instead use the Winforms Dispatcher, which will invoke the parent
Task on the "UI" thread but allow child Tasks to use the ThreadPool.
@KevinH-MS
Copy link
Contributor Author

FYI @dotnet/roslyn-interactive
/cc @cston

@@ -443,7 +443,7 @@ internal new Task<ScriptState<T>> ContinueAsync(ScriptState previousState, Cance

private async Task<ScriptState<T>> RunSubmissionsAsync(ScriptExecutionState executionState, ImmutableArray<Func<object[], Task>> precedingExecutors, Func<object[], Task> currentExecutor, CancellationToken cancellationToken)
{
var result = await executionState.RunSubmissionsAsync<T>(precedingExecutors, currentExecutor, cancellationToken).ConfigureAwait(continueOnCapturedContext: true);
var result = await executionState.RunSubmissionsAsync<T>(precedingExecutors, currentExecutor, cancellationToken).ConfigureAwait(continueOnCapturedContext: false);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not strictly necessary, but I can't imagine why we'd want to continue inline, and there's no comment. It seemed safer to change this to false.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @tmat

@amcasey
Copy link
Member

amcasey commented Jan 5, 2016

No objections. :)

@KevinH-MS KevinH-MS force-pushed the EliminateTaskScheduler branch from 865e056 to 83f21ab Compare January 5, 2016 19:14
KevinH-MS added a commit that referenced this pull request Jan 5, 2016
Avoid async deadlock in InteractiveHost...
@KevinH-MS KevinH-MS merged commit 8cba720 into dotnet:master Jan 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants