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

Commit 04f0b1e

Browse files
Priya91stephentoub
authored andcommitted
Make SslStreamInternal to free resources with SslStream dispose. (#26666)
* Make SslStreamInternal to free resources with SslStream dispose. * Fix framework test. * Respond to PR feedback.
1 parent 7c4d35b commit 04f0b1e

File tree

3 files changed

+79
-9
lines changed

3 files changed

+79
-9
lines changed

src/System.Net.Security/src/System/Net/Security/SslState.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -467,11 +467,9 @@ internal Task FlushAsync(CancellationToken cancellationToken)
467467
//
468468
internal void Close()
469469
{
470-
_exception = ExceptionDispatchInfo.Capture(new ObjectDisposedException("SslStream"));
471-
if (Context != null)
472-
{
473-
Context.Close();
474-
}
470+
_exception = ExceptionDispatchInfo.Capture(new ObjectDisposedException(nameof(SslStream)));
471+
Context?.Close();
472+
_secureStream?.Dispose();
475473
}
476474

477475
internal SecurityStatusPal EncryptData(ReadOnlyMemory<byte> buffer, ref byte[] outBuffer, out int outSize)

src/System.Net.Security/src/System/Net/Security/SslStreamInternal.cs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace System.Net.Security
1414
//
1515
// This is a wrapping stream that does data encryption/decryption based on a successfully authenticated SSPI context.
1616
//
17-
internal partial class SslStreamInternal
17+
internal partial class SslStreamInternal : IDisposable
1818
{
1919
private const int FrameOverhead = 32;
2020
private const int ReadBufferSize = 4096 * 4 + FrameOverhead; // We read in 16K chunks + headers.
@@ -57,10 +57,36 @@ private void ReturnReadBufferIfEmpty()
5757

5858
~SslStreamInternal()
5959
{
60-
if (_internalBuffer != null)
60+
Dispose(disposing: false);
61+
}
62+
63+
public void Dispose()
64+
{
65+
Dispose(disposing: true);
66+
67+
if (_internalBuffer == null)
6168
{
62-
ArrayPool<byte>.Shared.Return(_internalBuffer);
63-
_internalBuffer = null;
69+
// Suppress finalizer if the read buffer was returned.
70+
GC.SuppressFinalize(this);
71+
}
72+
}
73+
74+
private void Dispose(bool disposing)
75+
{
76+
// Ensure a Read operation is not in progress,
77+
// block potential reads since SslStream is disposing.
78+
// This leaves the _nestedRead = 1, but that's ok, since
79+
// subsequent Reads first check if the context is still available.
80+
if (Interlocked.CompareExchange(ref _nestedRead, 1, 0) == 0)
81+
{
82+
byte[] buffer = _internalBuffer;
83+
if (buffer != null)
84+
{
85+
_internalBuffer = null;
86+
_internalBufferCount = 0;
87+
_internalOffset = 0;
88+
ArrayPool<byte>.Shared.Return(buffer);
89+
}
6490
}
6591
}
6692

src/System.Net.Security/tests/FunctionalTests/SslStreamStreamToStreamTest.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
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+
using System.IO;
56
using System.Linq;
67
using System.Net.Test.Common;
78
using System.Security.Authentication;
@@ -306,6 +307,51 @@ await serverSslStream.WriteAsync(new byte[] { 1 }, 0, 1)
306307
}
307308
}
308309

310+
[Fact]
311+
public async Task SslStream_StreamToStream_Dispose_Throws()
312+
{
313+
VirtualNetwork network = new VirtualNetwork();
314+
using (var clientStream = new VirtualNetworkStream(network, isServer: false))
315+
using (var serverStream = new VirtualNetworkStream(network, isServer: true))
316+
using (var clientSslStream = new SslStream(clientStream, false, AllowAnyServerCertificate))
317+
{
318+
var serverSslStream = new SslStream(serverStream);
319+
await DoHandshake(clientSslStream, serverSslStream);
320+
321+
var serverBuffer = new byte[1];
322+
Task serverReadTask = serverSslStream.ReadAsync(serverBuffer, 0, serverBuffer.Length);
323+
await serverSslStream.WriteAsync(new byte[] { 1 }, 0, 1)
324+
.TimeoutAfter(TestConfiguration.PassingTestTimeoutMilliseconds);
325+
326+
// Shouldn't throw, the context is diposed now.
327+
// Since the server read task is in progress, the read buffer is not returned to ArrayPool.
328+
serverSslStream.Dispose();
329+
330+
// Read in client
331+
var clientBuffer = new byte[1];
332+
await clientSslStream.ReadAsync(clientBuffer, 0, clientBuffer.Length);
333+
Assert.Equal(1, clientBuffer[0]);
334+
335+
await clientSslStream.WriteAsync(new byte[] { 2 }, 0, 1);
336+
337+
if (PlatformDetection.IsFullFramework)
338+
{
339+
await Assert.ThrowsAsync<ObjectDisposedException>(() => serverReadTask);
340+
}
341+
else
342+
{
343+
IOException serverException = await Assert.ThrowsAsync<IOException>(() => serverReadTask);
344+
Assert.IsType<ObjectDisposedException>(serverException.InnerException);
345+
}
346+
347+
await Assert.ThrowsAsync<ObjectDisposedException>(() => serverSslStream.ReadAsync(serverBuffer, 0, serverBuffer.Length));
348+
349+
// Now, there is no pending read, so the internal buffer will be returned to ArrayPool.
350+
serverSslStream.Dispose();
351+
await Assert.ThrowsAsync<ObjectDisposedException>(() => serverSslStream.ReadAsync(serverBuffer, 0, serverBuffer.Length));
352+
}
353+
}
354+
309355
[Fact]
310356
public void SslStream_StreamToStream_Flush_Propagated()
311357
{

0 commit comments

Comments
 (0)