Skip to content

Commit 9fdb790

Browse files
committed
Use configure await to yield to threadpool
1 parent 6b9142f commit 9fdb790

File tree

2 files changed

+59
-65
lines changed

2 files changed

+59
-65
lines changed

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

Lines changed: 58 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public SslConnectionOptions(QuicConnection connection, bool isClient,
6666
_certificateChainPolicy = certificateChainPolicy;
6767
}
6868

69-
internal unsafe void StartAsyncCertificateValidation(void* certificatePtr, void* chainPtr)
69+
internal async void StartAsyncCertificateValidation(IntPtr certificatePtr, IntPtr chainPtr)
7070
{
7171
//
7272
// The provided data pointers are valid only while still inside this function, so they need to be
@@ -80,96 +80,90 @@ internal unsafe void StartAsyncCertificateValidation(void* certificatePtr, void*
8080
byte[]? chainDataRented = null;
8181
Memory<byte> chainData = default;
8282

83-
if (certificatePtr != null)
83+
if (certificatePtr != IntPtr.Zero)
8484
{
8585
if (MsQuicApi.UsesSChannelBackend)
8686
{
87-
certificate = new X509Certificate2((IntPtr)certificatePtr);
87+
// provided data is a pointer to a CERT_CONTEXT
88+
certificate = new X509Certificate2(certificatePtr);
8889
// TODO: what about chainPtr?
8990
}
9091
else
9192
{
92-
// On non-SChannel backends we specify USE_PORTABLE_CERTIFICATES and the content is buffers
93-
// with DER encoded cert and chain.
94-
QUIC_BUFFER* certificateBuffer = (QUIC_BUFFER*)certificatePtr;
95-
QUIC_BUFFER* chainBuffer = (QUIC_BUFFER*)chainPtr;
96-
97-
if (certificateBuffer->Length > 0)
93+
unsafe
9894
{
99-
certDataRented = ArrayPool<byte>.Shared.Rent((int)certificateBuffer->Length);
100-
certData = certDataRented.AsMemory(0, (int)certificateBuffer->Length);
101-
certificateBuffer->Span.CopyTo(certData.Span);
102-
}
95+
// On non-SChannel backends we specify USE_PORTABLE_CERTIFICATES and the contents are buffers
96+
// with DER encoded cert and chain.
97+
QUIC_BUFFER* certificateBuffer = (QUIC_BUFFER*)certificatePtr;
98+
QUIC_BUFFER* chainBuffer = (QUIC_BUFFER*)chainPtr;
10399

104-
if (chainBuffer->Length > 0)
105-
{
106-
chainDataRented = ArrayPool<byte>.Shared.Rent((int)chainBuffer->Length);
107-
chainData = chainDataRented.AsMemory(0, (int)chainBuffer->Length);
108-
chainBuffer->Span.CopyTo(chainData.Span);
100+
if (certificateBuffer->Length > 0)
101+
{
102+
certDataRented = ArrayPool<byte>.Shared.Rent((int)certificateBuffer->Length);
103+
certData = certDataRented.AsMemory(0, (int)certificateBuffer->Length);
104+
certificateBuffer->Span.CopyTo(certData.Span);
105+
}
106+
107+
if (chainBuffer->Length > 0)
108+
{
109+
chainDataRented = ArrayPool<byte>.Shared.Rent((int)chainBuffer->Length);
110+
chainData = chainDataRented.AsMemory(0, (int)chainBuffer->Length);
111+
chainBuffer->Span.CopyTo(chainData.Span);
112+
}
109113
}
110114
}
111115
}
112116

113-
QuicConnection connection = _connection;
117+
// We wan't to do the certificate validation asynchronously, but due to a bug in MsQuic, we need to call the callback synchronously on some versions
114118
if (MsQuicApi.SupportsAsyncCertValidation)
115119
{
116-
// hand-off rest of the work to the thread pool, certificatePtr and chainPtr are invalid beyond this point
117-
_ = Task.Run(() =>
118-
{
119-
StartAsyncCertificateValidationCore(connection, certificate, certData, chainData, certDataRented, chainDataRented);
120-
});
121-
}
122-
else
123-
{
124-
// due to a bug in MsQuic, we need to call the callback synchronously to close the connection properly when
125-
// we reject the certificate
126-
StartAsyncCertificateValidationCore(connection, certificate, certData, chainData, certDataRented, chainDataRented);
120+
// force yield to the thread pool to free up MsQuic worker thread.
121+
await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding);
127122
}
128123

129-
static void StartAsyncCertificateValidationCore(QuicConnection thisConnection, X509Certificate2? certificate, Memory<byte> certData, Memory<byte> chainData, byte[]? certDataRented, byte[]? chainDataRented)
124+
// certificatePtr and chainPtr are invalid beyond this point
125+
126+
QUIC_TLS_ALERT_CODES result;
127+
try
130128
{
131-
QUIC_TLS_ALERT_CODES result;
132-
try
129+
if (certData.Length > 0)
133130
{
134-
if (certData.Length > 0)
135-
{
136-
Debug.Assert(certificate == null);
137-
certificate = new X509Certificate2(certData.Span);
138-
}
139-
140-
result = thisConnection._sslConnectionOptions.ValidateCertificate(certificate, certData.Span, chainData.Span);
141-
thisConnection._remoteCertificate = certificate;
131+
Debug.Assert(certificate == null);
132+
certificate = new X509Certificate2(certData.Span);
142133
}
143-
catch (Exception ex)
134+
135+
result = _connection._sslConnectionOptions.ValidateCertificate(certificate, certData.Span, chainData.Span);
136+
_connection._remoteCertificate = certificate;
137+
}
138+
catch (Exception ex)
139+
{
140+
certificate?.Dispose();
141+
_connection._connectedTcs.TrySetException(ex);
142+
result = QUIC_TLS_ALERT_CODES.USER_CANCELED;
143+
}
144+
finally
145+
{
146+
if (certDataRented != null)
144147
{
145-
certificate?.Dispose();
146-
thisConnection._connectedTcs.TrySetException(ex);
147-
result = QUIC_TLS_ALERT_CODES.USER_CANCELED;
148+
ArrayPool<byte>.Shared.Return(certDataRented);
148149
}
149-
finally
150-
{
151-
if (certDataRented != null)
152-
{
153-
ArrayPool<byte>.Shared.Return(certDataRented);
154-
}
155150

156-
if (chainDataRented != null)
157-
{
158-
ArrayPool<byte>.Shared.Return(chainDataRented);
159-
}
151+
if (chainDataRented != null)
152+
{
153+
ArrayPool<byte>.Shared.Return(chainDataRented);
160154
}
155+
}
161156

162-
int status = MsQuicApi.Api.ConnectionCertificateValidationComplete(
163-
thisConnection._handle,
164-
result == QUIC_TLS_ALERT_CODES.SUCCESS ? (byte)1 : (byte)0,
165-
result);
157+
int status = MsQuicApi.Api.ConnectionCertificateValidationComplete(
158+
_connection._handle,
159+
result == QUIC_TLS_ALERT_CODES.SUCCESS ? (byte)1 : (byte)0,
160+
result);
166161

167-
if (MsQuic.StatusFailed(status))
162+
if (MsQuic.StatusFailed(status))
163+
{
164+
if (NetEventSource.Log.IsEnabled())
168165
{
169-
if (NetEventSource.Log.IsEnabled())
170-
{
171-
NetEventSource.Error(thisConnection, $"{thisConnection} ConnectionCertificateValidationComplete failed with {ThrowHelper.GetErrorMessageForStatus(status)}");
172-
}
166+
NetEventSource.Error(_connection, $"{_connection} ConnectionCertificateValidationComplete failed with {ThrowHelper.GetErrorMessageForStatus(status)}");
173167
}
174168
}
175169
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEI
578578
// worker threads.
579579
//
580580

581-
_sslConnectionOptions.StartAsyncCertificateValidation(data.Certificate, data.Chain);
581+
_sslConnectionOptions.StartAsyncCertificateValidation((IntPtr)data.Certificate, (IntPtr)data.Chain);
582582
return QUIC_STATUS_PENDING;
583583
}
584584

0 commit comments

Comments
 (0)