Add Delegate cast to Task constructors #5765
Conversation
|
I would be nice to fix this by making the code more similar to the (more optimized) CoreCLR version: https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Threading/Tasks/Task.cs . The CoreCLR version does not have |
|
Could you please delete |
| @@ -590,6 +581,8 @@ public Task(Action<object> action, object state, CancellationToken cancellationT | |||
|
|
|||
| AssignCancellationToken(cancellationToken, null, null); | |||
| } | |||
|
|
|||
| PossiblyCaptureContext(); | |||
There was a problem hiding this comment.
I would inline it here, so that it looks just like CoreCLR version. It is not really worth it to have it in a separate function when it is called just from a single place.
There was a problem hiding this comment.
Should it really be called from there now that it is called from the core Task constructor?
Note that Task<T> inherits from Task.
| @@ -476,14 +469,12 @@ public Task(Action<object> action, object state, TaskCreationOptions creationOpt | |||
| public Task(Action<object> action, object state, CancellationToken cancellationToken, TaskCreationOptions creationOptions) | |||
| : this(action, state, Task.InternalCurrentIfAttached(creationOptions), cancellationToken, creationOptions, InternalTaskOptions.None, null) | |||
| { | |||
| PossiblyCaptureContext(); | |||
| } | |||
|
|
|||
| internal Task(Action<object> action, object state, Task parent, CancellationToken cancellationToken, | |||
There was a problem hiding this comment.
This constructor can be deleted now, I think.
A-And
changed the title
Running runtest.cmd /corefx on Windows with the
System.Threading.Testsin the CoreFX test list hits the following assertioncorert/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs
Line 792 in becb08e
The assertion is caused by a Task constructors calling an overloaded constructor, both of which call PossiblyCaptureContext in their body. Is this intentional?
Adding the cast resolves the constructor to this one
corert/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs
Line 500 in becb08e
This seems to fix the problem without glaring regressions, including allowing
System.Threading.Teststo run under Windows.