Skip to content

Commit

Permalink
Fix AwaitableSocketAsyncEventArgs reorderings on weaker memory models (…
Browse files Browse the repository at this point in the history 
…#84641)

There are a couple of places where we read the _continuation field and then read some other state which we assume to be consistent with the value we read in _continuation.  But without a fence, those secondary reads could be reordered with respect to the first.

Co-authored-by: Stephen Toub <[email protected]>
  • Loading branch information
@github-actions @stephentoub
github-actions[bot] and stephentoub authored Apr 14, 2023
1 parent 2165458 commit 70c91e2
Showing 1 changed file with 11 additions and 11 deletions.
22 changes: 11 additions & 11 deletions src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ internal sealed class AwaitableSocketAsyncEventArgs : SocketAsyncEventArgs, IVal
/// <see cref="s_completedSentinel"/> if it has completed. Another delegate if OnCompleted was called before the operation could complete,
/// in which case it's the delegate to invoke when the operation does complete.
/// </summary>
private Action<object?>? _continuation;
private volatile Action<object?>? _continuation;
private ExecutionContext? _executionContext;
private object? _scheduler;
/// <summary>Current token value given to a ValueTask and then verified against the value it passes back to us.</summary>
Expand Down Expand Up @@ -1007,7 +1007,7 @@ protected override void OnCompleted(SocketAsyncEventArgs _)
/// <returns>This instance.</returns>
public ValueTask<Socket> AcceptAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.AcceptAsync(this, cancellationToken))
{
Expand All @@ -1031,7 +1031,7 @@ public ValueTask<Socket> AcceptAsync(Socket socket, CancellationToken cancellati
/// <returns>This instance.</returns>
public ValueTask<int> ReceiveAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.ReceiveAsync(this, cancellationToken))
{
Expand All @@ -1051,7 +1051,7 @@ public ValueTask<int> ReceiveAsync(Socket socket, CancellationToken cancellation

public ValueTask<SocketReceiveFromResult> ReceiveFromAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.ReceiveFromAsync(this, cancellationToken))
{
Expand All @@ -1072,7 +1072,7 @@ public ValueTask<SocketReceiveFromResult> ReceiveFromAsync(Socket socket, Cancel

public ValueTask<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.ReceiveMessageFromAsync(this, cancellationToken))
{
Expand All @@ -1097,7 +1097,7 @@ public ValueTask<SocketReceiveMessageFromResult> ReceiveMessageFromAsync(Socket
/// <returns>This instance.</returns>
public ValueTask<int> SendAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.SendAsync(this, cancellationToken))
{
Expand All @@ -1117,7 +1117,7 @@ public ValueTask<int> SendAsync(Socket socket, CancellationToken cancellationTok

public ValueTask SendAsyncForNetworkStream(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.SendAsync(this, cancellationToken))
{
Expand All @@ -1136,7 +1136,7 @@ public ValueTask SendAsyncForNetworkStream(Socket socket, CancellationToken canc

public ValueTask SendPacketsAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.SendPacketsAsync(this, cancellationToken))
{
Expand All @@ -1155,7 +1155,7 @@ public ValueTask SendPacketsAsync(Socket socket, CancellationToken cancellationT

public ValueTask<int> SendToAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

if (socket.SendToAsync(this, cancellationToken))
{
Expand All @@ -1175,7 +1175,7 @@ public ValueTask<int> SendToAsync(Socket socket, CancellationToken cancellationT

public ValueTask ConnectAsync(Socket socket)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, "Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, "Expected null continuation to indicate reserved for use");

try
{
Expand All @@ -1201,7 +1201,7 @@ public ValueTask ConnectAsync(Socket socket)

public ValueTask DisconnectAsync(Socket socket, CancellationToken cancellationToken)
{
Debug.Assert(Volatile.Read(ref _continuation) == null, $"Expected null continuation to indicate reserved for use");
Debug.Assert(_continuation == null, $"Expected null continuation to indicate reserved for use");

if (socket.DisconnectAsync(this, cancellationToken))
{
Expand Down

0 comments on commit 70c91e2

Please sign in to comment.