Skip to content

Commit

Permalink
Fix ExecutionContext capture in Task (dotnet#17407)
Browse files Browse the repository at this point in the history
When Task was refactored in core to remove the defunct StackCrawlMark support, the code that captured ExecutionContext was moved from a helper into TaskConstructorCore.  In doing so, though, it was put at the beginning of TaskConstructorCore, even though in its previous location it would have come after the call to TaskConstructorCore.

That change of place is causing an assert to fire, validating that m_stateFlags is still 0, which is failing because if ExecutionContext.Capture() returns null due to flow being suppressed, the code that stores the context sets a bit into m_stateFlags (to be able to differentiate null meaning no state flowed and null meaning default).

The assert is correct, though, and a small bug did result from this: Task hasn't been entirely respecting ExecutionContext.SuppressFlow (which was added back in .NET Core 2.0), in that it won't flow the current context, but because it would overwrite that bit in m_stateFlags, it would treat a null ExecutionContext as default instead of as flow suppressed, and thus would use ExecutionContext.Default to invoke the Task's delegate.  That in turn means that any EC changes made by the Task's delegate wouldn't be visible to the caller, as EC.Run would be used.

The fix is simply to move the code to the end of TaskConstructorCore instead of having it at the beginning.
  • Loading branch information
stephentoub authored Apr 4, 2018
1 parent b71cdf1 commit 9f41675
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/mscorlib/src/System/Threading/Tasks/Task.cs
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,6 @@ internal void TaskConstructorCore(Delegate action, object state, CancellationTok
m_stateObject = state;
m_taskScheduler = scheduler;

Debug.Assert(m_contingentProperties == null || m_contingentProperties.m_capturedContext == null,
"Captured an ExecutionContext when one was already captured.");
CapturedContext = ExecutionContext.Capture();

// Check for validity of options
if ((creationOptions &
~(TaskCreationOptions.AttachedToParent |
Expand Down Expand Up @@ -602,6 +598,10 @@ internal void TaskConstructorCore(Delegate action, object state, CancellationTok

AssignCancellationToken(cancellationToken, null, null);
}

Debug.Assert(m_contingentProperties == null || m_contingentProperties.m_capturedContext == null,
"Captured an ExecutionContext when one was already captured.");
CapturedContext = ExecutionContext.Capture();
}

/// <summary>
Expand Down

0 comments on commit 9f41675

Please sign in to comment.