Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 1b643bd

Browse files
dotnet-botahsonkhan
authored andcommitted
Mirror changes from dotnet/coreclr (#29044)
* Avoid Unsafe.As usage in ValueTask that can break type safety (#17471) Unsafe.As yields a performance improvement over using a normal cast, and it's fine when ValueTask is used correctly, but if the ValueTask instance were to be stored into a field and multiple threads incorrectly raced to access it, a torn read/write could result in violating type safety due to ObjectIsTask reading the wrong value for the associated object. This commit changes the implementation to only use the single object field to determine which paths to take, rather than factoring in a second field that may not be in sync. Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com> * Fix Assert in ValueTask (dotnet/coreclr#17511) Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com> * Add GetPinnableReference back to Span and ReadOnlySpan (#17504) Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com> * Fix CoreRT build breaks Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com> Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com> * Remove MemoryManager.Length (dotnet/coreclr#17498) * Remove MemoryManager.Length * Feedback * XML comment nits Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>
1 parent 5411a5c commit 1b643bd

File tree

7 files changed

+354
-241
lines changed

7 files changed

+354
-241
lines changed

src/Common/src/CoreLib/System/Gen2GcCallback.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private void Setup(Func<object, bool> callback, object targetObj)
6767
}
6868

6969
// Resurrect ourselves by re-registering for finalization.
70-
if (!Environment.HasShutdownStarted && !AppDomain.CurrentDomain.IsFinalizingForUnload())
70+
if (!Environment.HasShutdownStarted)
7171
{
7272
GC.ReRegisterForFinalize(this);
7373
}

src/Common/src/CoreLib/System/ReadOnlySpan.Fast.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,13 @@ public ref readonly T this[int index]
149149
#endif
150150
}
151151

152+
/// <summary>
153+
/// Returns a reference to the 0th element of the Span. If the Span is empty, returns null reference.
154+
/// It can be used for pinning and is required to support the use of span within a fixed statement.
155+
/// </summary>
156+
[EditorBrowsable(EditorBrowsableState.Never)]
157+
public unsafe ref readonly T GetPinnableReference() => ref (_length != 0) ? ref _pointer.Value : ref Unsafe.AsRef<T>(null);
158+
152159
/// <summary>
153160
/// Copies the contents of this read-only span into destination span. If the source
154161
/// and destinations overlap, this method behaves as if the original values in

src/Common/src/CoreLib/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs

Lines changed: 54 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -59,55 +59,64 @@ public bool IsCompleted
5959
/// <summary>Schedules the continuation action for the <see cref="ConfiguredValueTaskAwaitable"/>.</summary>
6060
public void OnCompleted(Action continuation)
6161
{
62-
if (_value.ObjectIsTask)
62+
object obj = _value._obj;
63+
Debug.Assert(obj == null || obj is Task || obj is IValueTaskSource);
64+
65+
if (obj is Task t)
6366
{
64-
_value.UnsafeGetTask().ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
67+
t.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
6568
}
66-
else if (_value._obj != null)
69+
else if (obj != null)
6770
{
68-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
71+
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
6972
ValueTaskSourceOnCompletedFlags.FlowExecutionContext |
70-
(_value.ContinueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None));
73+
(_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None));
7174
}
7275
else
7376
{
74-
ValueTask.CompletedTask.ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
77+
ValueTask.CompletedTask.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
7578
}
7679
}
7780

7881
/// <summary>Schedules the continuation action for the <see cref="ConfiguredValueTaskAwaitable"/>.</summary>
7982
public void UnsafeOnCompleted(Action continuation)
8083
{
81-
if (_value.ObjectIsTask)
84+
object obj = _value._obj;
85+
Debug.Assert(obj == null || obj is Task || obj is IValueTaskSource);
86+
87+
if (obj is Task t)
8288
{
83-
_value.UnsafeGetTask().ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
89+
t.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
8490
}
85-
else if (_value._obj != null)
91+
else if (obj != null)
8692
{
87-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
88-
_value.ContinueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
93+
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
94+
_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
8995
}
9096
else
9197
{
92-
ValueTask.CompletedTask.ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
98+
ValueTask.CompletedTask.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
9399
}
94100
}
95101

96102
#if CORECLR
97103
void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox box)
98104
{
99-
if (_value.ObjectIsTask)
105+
object obj = _value._obj;
106+
Debug.Assert(obj == null || obj is Task || obj is IValueTaskSource);
107+
108+
if (obj is Task t)
100109
{
101-
TaskAwaiter.UnsafeOnCompletedInternal(_value.UnsafeGetTask(), box, _value.ContinueOnCapturedContext);
110+
TaskAwaiter.UnsafeOnCompletedInternal(t, box, _value._continueOnCapturedContext);
102111
}
103-
else if (_value._obj != null)
112+
else if (obj != null)
104113
{
105-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeAsyncStateMachineBox, box, _value._token,
106-
_value.ContinueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
114+
Unsafe.As<IValueTaskSource>(obj).OnCompleted(ValueTaskAwaiter.s_invokeAsyncStateMachineBox, box, _value._token,
115+
_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
107116
}
108117
else
109118
{
110-
TaskAwaiter.UnsafeOnCompletedInternal(Task.CompletedTask, box, _value.ContinueOnCapturedContext);
119+
TaskAwaiter.UnsafeOnCompletedInternal(Task.CompletedTask, box, _value._continueOnCapturedContext);
111120
}
112121
}
113122
#endif
@@ -161,55 +170,64 @@ public bool IsCompleted
161170
/// <summary>Schedules the continuation action for the <see cref="ConfiguredValueTaskAwaitable{TResult}"/>.</summary>
162171
public void OnCompleted(Action continuation)
163172
{
164-
if (_value.ObjectIsTask)
173+
object obj = _value._obj;
174+
Debug.Assert(obj == null || obj is Task<TResult> || obj is IValueTaskSource<TResult>);
175+
176+
if (obj is Task<TResult> t)
165177
{
166-
_value.UnsafeGetTask().ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
178+
t.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
167179
}
168-
else if (_value._obj != null)
180+
else if (obj != null)
169181
{
170-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
182+
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
171183
ValueTaskSourceOnCompletedFlags.FlowExecutionContext |
172-
(_value.ContinueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None));
184+
(_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None));
173185
}
174186
else
175187
{
176-
ValueTask.CompletedTask.ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
188+
ValueTask.CompletedTask.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().OnCompleted(continuation);
177189
}
178190
}
179191

180192
/// <summary>Schedules the continuation action for the <see cref="ConfiguredValueTaskAwaitable{TResult}"/>.</summary>
181193
public void UnsafeOnCompleted(Action continuation)
182194
{
183-
if (_value.ObjectIsTask)
195+
object obj = _value._obj;
196+
Debug.Assert(obj == null || obj is Task<TResult> || obj is IValueTaskSource<TResult>);
197+
198+
if (obj is Task<TResult> t)
184199
{
185-
_value.UnsafeGetTask().ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
200+
t.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
186201
}
187-
else if (_value._obj != null)
202+
else if (obj != null)
188203
{
189-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
190-
_value.ContinueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
204+
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token,
205+
_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
191206
}
192207
else
193208
{
194-
ValueTask.CompletedTask.ConfigureAwait(_value.ContinueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
209+
ValueTask.CompletedTask.ConfigureAwait(_value._continueOnCapturedContext).GetAwaiter().UnsafeOnCompleted(continuation);
195210
}
196211
}
197212

198213
#if CORECLR
199214
void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox box)
200215
{
201-
if (_value.ObjectIsTask)
216+
object obj = _value._obj;
217+
Debug.Assert(obj == null || obj is Task<TResult> || obj is IValueTaskSource<TResult>);
218+
219+
if (obj is Task<TResult> t)
202220
{
203-
TaskAwaiter.UnsafeOnCompletedInternal(_value.UnsafeGetTask(), box, _value.ContinueOnCapturedContext);
221+
TaskAwaiter.UnsafeOnCompletedInternal(t, box, _value._continueOnCapturedContext);
204222
}
205-
else if (_value._obj != null)
223+
else if (obj != null)
206224
{
207-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeAsyncStateMachineBox, box, _value._token,
208-
_value.ContinueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
225+
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ValueTaskAwaiter.s_invokeAsyncStateMachineBox, box, _value._token,
226+
_value._continueOnCapturedContext ? ValueTaskSourceOnCompletedFlags.UseSchedulingContext : ValueTaskSourceOnCompletedFlags.None);
209227
}
210228
else
211229
{
212-
TaskAwaiter.UnsafeOnCompletedInternal(Task.CompletedTask, box, _value.ContinueOnCapturedContext);
230+
TaskAwaiter.UnsafeOnCompletedInternal(Task.CompletedTask, box, _value._continueOnCapturedContext);
213231
}
214232
}
215233
#endif

src/Common/src/CoreLib/System/Runtime/CompilerServices/ValueTaskAwaiter.cs

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
using System.Threading.Tasks;
77
using System.Threading.Tasks.Sources;
88

9+
#if !netstandard
10+
using Internal.Runtime.CompilerServices;
11+
#endif
12+
913
namespace System.Runtime.CompilerServices
1014
{
1115
/// <summary>Provides an awaiter for a <see cref="ValueTask"/>.</summary>
@@ -48,13 +52,16 @@ public bool IsCompleted
4852
/// <summary>Schedules the continuation action for this ValueTask.</summary>
4953
public void OnCompleted(Action continuation)
5054
{
51-
if (_value.ObjectIsTask)
55+
object obj = _value._obj;
56+
Debug.Assert(obj == null || obj is Task || obj is IValueTaskSource);
57+
58+
if (obj is Task t)
5259
{
53-
_value.UnsafeGetTask().GetAwaiter().OnCompleted(continuation);
60+
t.GetAwaiter().OnCompleted(continuation);
5461
}
55-
else if (_value._obj != null)
62+
else if (obj != null)
5663
{
57-
_value.UnsafeGetValueTaskSource().OnCompleted(s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext | ValueTaskSourceOnCompletedFlags.FlowExecutionContext);
64+
Unsafe.As<IValueTaskSource>(obj).OnCompleted(s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext | ValueTaskSourceOnCompletedFlags.FlowExecutionContext);
5865
}
5966
else
6067
{
@@ -65,13 +72,16 @@ public void OnCompleted(Action continuation)
6572
/// <summary>Schedules the continuation action for this ValueTask.</summary>
6673
public void UnsafeOnCompleted(Action continuation)
6774
{
68-
if (_value.ObjectIsTask)
75+
object obj = _value._obj;
76+
Debug.Assert(obj == null || obj is Task || obj is IValueTaskSource);
77+
78+
if (obj is Task t)
6979
{
70-
_value.UnsafeGetTask().GetAwaiter().UnsafeOnCompleted(continuation);
80+
t.GetAwaiter().UnsafeOnCompleted(continuation);
7181
}
72-
else if (_value._obj != null)
82+
else if (obj != null)
7383
{
74-
_value.UnsafeGetValueTaskSource().OnCompleted(s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
84+
Unsafe.As<IValueTaskSource>(obj).OnCompleted(s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
7585
}
7686
else
7787
{
@@ -82,13 +92,16 @@ public void UnsafeOnCompleted(Action continuation)
8292
#if CORECLR
8393
void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox box)
8494
{
85-
if (_value.ObjectIsTask)
95+
object obj = _value._obj;
96+
Debug.Assert(obj == null || obj is Task || obj is IValueTaskSource);
97+
98+
if (obj is Task t)
8699
{
87-
TaskAwaiter.UnsafeOnCompletedInternal(_value.UnsafeGetTask(), box, continueOnCapturedContext: true);
100+
TaskAwaiter.UnsafeOnCompletedInternal(t, box, continueOnCapturedContext: true);
88101
}
89-
else if (_value._obj != null)
102+
else if (obj != null)
90103
{
91-
_value.UnsafeGetValueTaskSource().OnCompleted(s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
104+
Unsafe.As<IValueTaskSource>(obj).OnCompleted(s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
92105
}
93106
else
94107
{
@@ -139,13 +152,16 @@ public bool IsCompleted
139152
/// <summary>Schedules the continuation action for this ValueTask.</summary>
140153
public void OnCompleted(Action continuation)
141154
{
142-
if (_value.ObjectIsTask)
155+
object obj = _value._obj;
156+
Debug.Assert(obj == null || obj is Task<TResult> || obj is IValueTaskSource<TResult>);
157+
158+
if (obj is Task<TResult> t)
143159
{
144-
_value.UnsafeGetTask().GetAwaiter().OnCompleted(continuation);
160+
t.GetAwaiter().OnCompleted(continuation);
145161
}
146-
else if (_value._obj != null)
162+
else if (obj != null)
147163
{
148-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext | ValueTaskSourceOnCompletedFlags.FlowExecutionContext);
164+
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext | ValueTaskSourceOnCompletedFlags.FlowExecutionContext);
149165
}
150166
else
151167
{
@@ -156,13 +172,16 @@ public void OnCompleted(Action continuation)
156172
/// <summary>Schedules the continuation action for this ValueTask.</summary>
157173
public void UnsafeOnCompleted(Action continuation)
158174
{
159-
if (_value.ObjectIsTask)
175+
object obj = _value._obj;
176+
Debug.Assert(obj == null || obj is Task<TResult> || obj is IValueTaskSource<TResult>);
177+
178+
if (obj is Task<TResult> t)
160179
{
161-
_value.UnsafeGetTask().GetAwaiter().UnsafeOnCompleted(continuation);
180+
t.GetAwaiter().UnsafeOnCompleted(continuation);
162181
}
163-
else if (_value._obj != null)
182+
else if (obj != null)
164183
{
165-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
184+
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ValueTaskAwaiter.s_invokeActionDelegate, continuation, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
166185
}
167186
else
168187
{
@@ -173,13 +192,16 @@ public void UnsafeOnCompleted(Action continuation)
173192
#if CORECLR
174193
void IStateMachineBoxAwareAwaiter.AwaitUnsafeOnCompleted(IAsyncStateMachineBox box)
175194
{
176-
if (_value.ObjectIsTask)
195+
object obj = _value._obj;
196+
Debug.Assert(obj == null || obj is Task<TResult> || obj is IValueTaskSource<TResult>);
197+
198+
if (obj is Task<TResult> t)
177199
{
178-
TaskAwaiter.UnsafeOnCompletedInternal(_value.UnsafeGetTask(), box, continueOnCapturedContext: true);
200+
TaskAwaiter.UnsafeOnCompletedInternal(t, box, continueOnCapturedContext: true);
179201
}
180-
else if (_value._obj != null)
202+
else if (obj != null)
181203
{
182-
_value.UnsafeGetValueTaskSource().OnCompleted(ValueTaskAwaiter.s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
204+
Unsafe.As<IValueTaskSource<TResult>>(obj).OnCompleted(ValueTaskAwaiter.s_invokeAsyncStateMachineBox, box, _value._token, ValueTaskSourceOnCompletedFlags.UseSchedulingContext);
183205
}
184206
else
185207
{

src/Common/src/CoreLib/System/Runtime/ConstrainedExecution/CriticalFinalizerObject.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,16 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
//
65
/*============================================================
76
**
87
**
9-
**
108
** Deriving from this class will cause any finalizer you define to be critical
119
** (i.e. the finalizer is guaranteed to run, won't be aborted by the host and is
1210
** run after the finalizers of other objects collected at the same time).
1311
**
14-
**
1512
**
1613
===========================================================*/
1714

18-
using System;
19-
using System.Runtime.InteropServices;
20-
2115
namespace System.Runtime.ConstrainedExecution
2216
{
2317
public abstract class CriticalFinalizerObject
@@ -26,6 +20,7 @@ protected CriticalFinalizerObject()
2620
{
2721
}
2822

23+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1821:RemoveEmptyFinalizers")]
2924
~CriticalFinalizerObject()
3025
{
3126
}

0 commit comments

Comments
 (0)