Skip to content

Commit 995c5e8

Browse files
committed
Use RVTS
1 parent c04ec4b commit 995c5e8

File tree

2 files changed

+38
-62
lines changed

2 files changed

+38
-62
lines changed

src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ValueTaskSource.cs

Lines changed: 15 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,13 @@ private enum State : byte
2525
private State _state;
2626
private ManualResetValueTaskSourceCore<bool> _valueTaskSource;
2727
private CancellationTokenRegistration _cancellationRegistration;
28-
private Exception? _exception;
2928
private GCHandle _keepAlive;
3029

3130
public ValueTaskSource()
3231
{
3332
_state = State.None;
3433
_valueTaskSource = new ManualResetValueTaskSourceCore<bool>() { RunContinuationsAsynchronously = true };
3534
_cancellationRegistration = default;
36-
_exception = default;
3735
_keepAlive = default;
3836
}
3937

@@ -77,7 +75,7 @@ public bool TryInitialize(out ValueTask valueTask, object? keepAlive = null, Can
7775
State state = _state;
7876

7977
// If we're the first here, we will return true.
80-
if (state == State.None && _valueTaskSource.GetStatus(_valueTaskSource.Version) == ValueTaskSourceStatus.Pending)
78+
if (state == State.None)
8179
{
8280
// Keep alive the caller object until the result is read from the task.
8381
// Used for keeping caller alive during async interop calls.
@@ -106,42 +104,30 @@ private bool TryComplete(Exception? exception)
106104
{
107105
State state = _state;
108106

109-
// Completed: nothing to do.
110-
if (state == State.Completed)
107+
if (state != State.Completed)
111108
{
112-
return false;
113-
}
114-
115-
// With cancellation, keep the state as-is so it can be restored after the OCE is consumed.
116-
_state = exception is OperationCanceledException ? state : State.Completed;
109+
_state = State.Completed;
117110

118-
// Swap the cancellation registration so the one that's been registered gets eventually Disposed.
119-
// Ideally, we would dispose it here, but if the callbacks kicks in, it tries to take the lock held by this thread leading to deadlock.
120-
cancellationRegistration = _cancellationRegistration;
121-
_cancellationRegistration = default;
111+
// Swap the cancellation registration so the one that's been registered gets eventually Disposed.
112+
// Ideally, we would dispose it here, but if the callbacks kicks in, it tries to take the lock held by this thread leading to deadlock.
113+
cancellationRegistration = _cancellationRegistration;
114+
_cancellationRegistration = default;
122115

123-
if (exception is not null)
124-
{
125-
// Set up the exception stack trace for the caller.
126-
exception = exception.StackTrace is null ? ExceptionDispatchInfo.SetCurrentStackTrace(exception) : exception;
127-
if (_valueTaskSource.GetStatus(_valueTaskSource.Version) == ValueTaskSourceStatus.Pending)
116+
if (exception is not null)
128117
{
118+
// Set up the exception stack trace for the caller.
119+
exception = exception.StackTrace is null ? ExceptionDispatchInfo.SetCurrentStackTrace(exception) : exception;
129120
_valueTaskSource.SetException(exception);
130121
}
131-
else if (exception is not OperationCanceledException)
132-
{
133-
_exception = exception;
134-
}
135-
}
136-
else
137-
{
138-
if (_valueTaskSource.GetStatus(_valueTaskSource.Version) == ValueTaskSourceStatus.Pending)
122+
else
139123
{
140124
_valueTaskSource.SetResult(true);
141125
}
126+
127+
return true;
142128
}
143129

144-
return true;
130+
return false;
145131
}
146132
finally
147133
{
@@ -187,34 +173,5 @@ void IValueTaskSource.OnCompleted(Action<object?> continuation, object? state, s
187173
=> _valueTaskSource.OnCompleted(continuation, state, token, flags);
188174

189175
void IValueTaskSource.GetResult(short token)
190-
{
191-
try
192-
{
193-
_valueTaskSource.GetResult(token);
194-
}
195-
finally
196-
{
197-
lock (this)
198-
{
199-
State state = _state;
200-
201-
// In case of a cancellation, reset the task and set the stored results if necessary.
202-
if (_valueTaskSource.GetStatus(_valueTaskSource.Version) == ValueTaskSourceStatus.Canceled)
203-
{
204-
_valueTaskSource.Reset();
205-
if (state == State.Completed)
206-
{
207-
if (_exception is not null)
208-
{
209-
_valueTaskSource.SetException(_exception);
210-
}
211-
else
212-
{
213-
_valueTaskSource.SetResult(true);
214-
}
215-
}
216-
}
217-
}
218-
}
219-
}
176+
=> _valueTaskSource.GetResult(token);
220177
}

src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,26 @@ static async ValueTask<QuicConnection> StartConnectAsync(QuicClientConnectionOpt
109109
private int _disposed;
110110

111111
private readonly ValueTaskSource _connectedTcs = new ValueTaskSource();
112-
private readonly ValueTaskSource _shutdownTcs = new ValueTaskSource();
112+
private readonly ResettableValueTaskSource _shutdownTcs = new ResettableValueTaskSource()
113+
{
114+
CancellationAction = target =>
115+
{
116+
try
117+
{
118+
if (target is QuicConnection connection)
119+
{
120+
// The OCE will be propagated through stored CancellationToken in ResettableValueTaskSource.
121+
connection._shutdownTcs.TrySetResult();
122+
}
123+
}
124+
catch (ObjectDisposedException)
125+
{
126+
// We collided with a Dispose in another thread. This can happen
127+
// when using CancellationTokenSource.CancelAfter.
128+
// Ignore the exception
129+
}
130+
}
131+
};
113132

114133
private readonly CancellationTokenSource _shutdownTokenSource = new CancellationTokenSource();
115134

@@ -467,7 +486,7 @@ public ValueTask CloseAsync(long errorCode, CancellationToken cancellationToken
467486
{
468487
ObjectDisposedException.ThrowIf(_disposed == 1, this);
469488

470-
if (_shutdownTcs.TryInitialize(out ValueTask valueTask, this, cancellationToken))
489+
if (_shutdownTcs.TryGetValueTask(out ValueTask valueTask, this, cancellationToken))
471490
{
472491
unsafe
473492
{
@@ -626,7 +645,7 @@ public async ValueTask DisposeAsync()
626645
}
627646

628647
// Check if the connection has been shut down and if not, shut it down.
629-
if (_shutdownTcs.TryInitialize(out ValueTask valueTask, this))
648+
if (_shutdownTcs.TryGetValueTask(out ValueTask valueTask, this))
630649
{
631650
unsafe
632651
{
@@ -648,7 +667,7 @@ public async ValueTask DisposeAsync()
648667
}
649668

650669
// Wait for SHUTDOWN_COMPLETE, the last event, so that all resources can be safely released.
651-
await valueTask.ConfigureAwait(false);
670+
await _shutdownTcs.GetFinalTask(this).ConfigureAwait(false);
652671
Debug.Assert(_connectedTcs.IsCompleted);
653672
_handle.Dispose();
654673
_shutdownTokenSource.Dispose();

0 commit comments

Comments
 (0)