Skip to content

Commit 68242c7

Browse files
authored
fix pooled array leak (#88810)
* ntlm * test base * qpack * FileSys * JSON * interp tests * fix NTAuthentication leak * More in JSON * more tests * ntlmserver disposable * more tests * more tests * tar tests * feedback
1 parent dfe7a6c commit 68242c7

File tree

17 files changed

+89
-50
lines changed

17 files changed

+89
-50
lines changed

src/libraries/Common/tests/System/Net/Security/FakeNtlmServer.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ namespace System.Net.Security
2424
// and responses for unit test purposes. The validation checks the
2525
// structure of the messages, their integrity and use of specified
2626
// features (eg. MIC).
27-
internal class FakeNtlmServer
27+
internal class FakeNtlmServer : IDisposable
2828
{
2929
public FakeNtlmServer(NetworkCredential expectedCredential)
3030
{
@@ -142,6 +142,14 @@ private enum AvFlags : uint
142142
UntrustedSPN = 4,
143143
}
144144

145+
public void Dispose()
146+
{
147+
_clientSeal?.Dispose();
148+
_clientSeal = null;
149+
_serverSeal?.Dispose();
150+
_serverSeal = null;
151+
}
152+
145153
private static ReadOnlySpan<byte> GetField(ReadOnlySpan<byte> payload, int fieldOffset)
146154
{
147155
uint offset = BinaryPrimitives.ReadUInt32LittleEndian(payload.Slice(fieldOffset + 4));
@@ -396,6 +404,9 @@ private void ValidateAuthentication(byte[] incomingBlob)
396404

397405
public void ResetKeys()
398406
{
407+
_clientSeal?.Dispose();
408+
_serverSeal?.Dispose();
409+
399410
_clientSeal = new RC4(_clientSealingKey);
400411
_serverSeal = new RC4(_serverSealingKey);
401412
}

src/libraries/Common/tests/TestUtilities/System/IO/FileCleanupTestBase.cs

Lines changed: 15 additions & 15 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

44
using System.Buffers;
5+
using System.ComponentModel;
56
using System.Diagnostics;
67
using System.Runtime.CompilerServices;
78
using System.Text;
@@ -201,32 +202,31 @@ private unsafe string GetTestDirectoryActualCasing()
201202

202203
if (!handle.IsInvalid)
203204
{
204-
const int InitialBufferSize = 4096;
205-
char[]? buffer = ArrayPool<char>.Shared.Rent(InitialBufferSize);
206-
uint result = GetFinalPathNameByHandle(handle, buffer);
205+
char[] buffer = new char[4096];
206+
uint result;
207+
fixed (char* bufPtr = buffer)
208+
{
209+
result = Interop.Kernel32.GetFinalPathNameByHandle(handle, bufPtr, (uint)buffer.Length, Interop.Kernel32.FILE_NAME_NORMALIZED);
210+
}
211+
212+
if (result == 0)
213+
{
214+
throw new Win32Exception();
215+
}
216+
217+
Debug.Assert(result <= buffer.Length);
207218

208219
// Remove extended prefix
209220
int skip = PathInternal.IsExtended(buffer) ? 4 : 0;
210221

211-
return new string(
212-
buffer,
213-
skip,
214-
(int)result - skip);
222+
return new string(buffer, skip, (int)result - skip);
215223
}
216224
}
217225
catch { }
218226

219227
return TestDirectory;
220228
}
221229

222-
private unsafe uint GetFinalPathNameByHandle(SafeFileHandle handle, char[] buffer)
223-
{
224-
fixed (char* bufPtr = buffer)
225-
{
226-
return Interop.Kernel32.GetFinalPathNameByHandle(handle, bufPtr, (uint)buffer.Length, Interop.Kernel32.FILE_NAME_NORMALIZED);
227-
}
228-
}
229-
230230
protected string CreateTestDirectory(params string[] paths)
231231
{
232232
string dir = Path.Combine(paths);

src/libraries/Common/tests/Tests/System/Net/MultiArrayBufferTests.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public sealed class MultiArrayBufferTests
1616
[Fact]
1717
public void BasicTest()
1818
{
19-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
19+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
2020

2121
Assert.True(buffer.IsEmpty);
2222
Assert.True(buffer.ActiveMemory.IsEmpty);
@@ -98,7 +98,7 @@ public void AddByteByByteAndConsumeByteByByte_Success()
9898
{
9999
const int Size = 64 * 1024 + 1;
100100

101-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
101+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
102102

103103
for (int i = 0; i < Size; i++)
104104
{
@@ -124,7 +124,7 @@ public void AddSeveralBytesRepeatedlyAndConsumeSeveralBytesRepeatedly_Success()
124124
const int ByteCount = 7;
125125
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries
126126

127-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
127+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
128128

129129
for (int i = 0; i < RepeatCount; i++)
130130
{
@@ -156,7 +156,7 @@ public void AddSeveralBytesRepeatedlyAndConsumeSeveralBytesRepeatedly_UsingSlice
156156
const int ByteCount = 7;
157157
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries
158158

159-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
159+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
160160

161161
for (int i = 0; i < RepeatCount; i++)
162162
{
@@ -188,7 +188,7 @@ public void AddSeveralBytesRepeatedlyAndConsumeSeveralBytesRepeatedly_UsingSlice
188188
const int ByteCount = 7;
189189
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries
190190

191-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
191+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
192192

193193
for (int i = 0; i < RepeatCount; i++)
194194
{
@@ -221,7 +221,7 @@ public void CopyFromRepeatedlyAndCopyToRepeatedly_Success()
221221

222222
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries
223223

224-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
224+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
225225

226226
for (int i = 0; i < RepeatCount; i++)
227227
{
@@ -250,7 +250,7 @@ public void CopyFromRepeatedlyAndCopyToRepeatedly_LargeCopies_Success()
250250

251251
const int RepeatCount = 13;
252252

253-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
253+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
254254

255255
for (int i = 0; i < RepeatCount; i++)
256256
{
@@ -291,7 +291,7 @@ public void EmptyMultiMemoryTest()
291291
[Fact]
292292
public void EnsureAvailableSpaceTest()
293293
{
294-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
294+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
295295

296296
Assert.Equal(0, buffer.ActiveMemory.Length);
297297
Assert.Equal(0, buffer.AvailableMemory.Length);
@@ -423,7 +423,7 @@ public void EnsureAvailableSpaceTest()
423423
[Fact]
424424
public void EnsureAvailableSpaceUpToLimitTest()
425425
{
426-
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
426+
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);
427427

428428
Assert.Equal(0, buffer.ActiveMemory.Length);
429429
Assert.Equal(0, buffer.AvailableMemory.Length);

src/libraries/Common/tests/Tests/System/Net/aspnetcore/Http3/QPackDecoderTest.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
namespace System.Net.Http.Unit.Tests.QPack
1717
{
18-
public class QPackDecoderTests
18+
public class QPackDecoderTests : IDisposable
1919
{
2020
private const int MaxHeaderFieldSize = 8192;
2121

@@ -64,6 +64,11 @@ public QPackDecoderTests()
6464
_decoder = new QPackDecoder(MaxHeaderFieldSize);
6565
}
6666

67+
public void Dispose()
68+
{
69+
_decoder.Dispose();
70+
}
71+
6772
[Fact]
6873
public void DecodesIndexedHeaderField_StaticTableWithValue()
6974
{
@@ -318,7 +323,7 @@ private static void TestDecodeWithoutIndexing(byte[] encoded, KeyValuePair<strin
318323

319324
private static void TestDecode(byte[] encoded, KeyValuePair<string, string>[] expectedValues, bool expectDynamicTableEntry, int? bytesAtATime)
320325
{
321-
QPackDecoder decoder = new QPackDecoder(MaxHeaderFieldSize);
326+
using QPackDecoder decoder = new QPackDecoder(MaxHeaderFieldSize);
322327
TestHttpHeadersHandler handler = new TestHttpHeadersHandler();
323328

324329
// Read past header

src/libraries/Common/tests/Tests/System/Text/ValueStringBuilderTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ public void AsSpan_ReturnsCorrectValue_DoesntClearBuilder()
180180

181181
Assert.NotEqual(0, sb.Length);
182182
Assert.Equal(sb.Length, vsb.Length);
183+
Assert.Equal(sb.ToString(), vsb.ToString());
183184
}
184185

185186
[Fact]
@@ -275,6 +276,7 @@ public unsafe void Indexer()
275276
Assert.Equal('b', vsb[3]);
276277
vsb[3] = 'c';
277278
Assert.Equal('c', vsb[3]);
279+
vsb.Dispose();
278280
}
279281

280282
[Fact]
@@ -297,6 +299,7 @@ public void EnsureCapacity_IfBufferTimesTwoWins()
297299
builder.EnsureCapacity(33);
298300

299301
Assert.Equal(64, builder.Capacity);
302+
builder.Dispose();
300303
}
301304

302305
[Fact]
@@ -309,6 +312,7 @@ public void EnsureCapacity_NoAllocIfNotNeeded()
309312
builder.EnsureCapacity(16);
310313

311314
Assert.Equal(64, builder.Capacity);
315+
builder.Dispose();
312316
}
313317
}
314318
}

src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ public void UnixFileModes_RestrictiveParentDir(bool overwrite)
340340
AssertFileModeEquals(filePath, TestPermission1);
341341
}
342342

343-
[Fact]
343+
[ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
344344
public void LinkBeforeTarget()
345345
{
346346
using TempDirectory source = new TempDirectory();

src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.File.Tests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ public async Task UnixFileModes_RestrictiveParentDir_Async()
362362
AssertFileModeEquals(filePath, TestPermission1);
363363
}
364364

365-
[Fact]
365+
[ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
366366
public async Task LinkBeforeTargetAsync()
367367
{
368368
using TempDirectory source = new TempDirectory();

src/libraries/System.Memory/tests/MemoryPool/MemoryPool.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public static void MemoryPoolPin(int elementIndex)
9090
public static void MemoryPoolPinBadOffset(int elementIndex)
9191
{
9292
MemoryPool<int> pool = MemoryPool<int>.Shared;
93-
IMemoryOwner<int> block = pool.Rent(10);
93+
using IMemoryOwner<int> block = pool.Rent(10);
9494
Memory<int> memory = block.Memory;
9595
Span<int> sp = memory.Span;
9696
Assert.Equal(memory.Length, sp.Length);
@@ -101,7 +101,7 @@ public static void MemoryPoolPinBadOffset(int elementIndex)
101101
public static void MemoryPoolPinOffsetAtEnd()
102102
{
103103
MemoryPool<int> pool = MemoryPool<int>.Shared;
104-
IMemoryOwner<int> block = pool.Rent(10);
104+
using IMemoryOwner<int> block = pool.Rent(10);
105105
Memory<int> memory = block.Memory;
106106
Span<int> sp = memory.Span;
107107
Assert.Equal(memory.Length, sp.Length);
@@ -122,7 +122,7 @@ public static void MemoryPoolPinOffsetAtEnd()
122122
public static void MemoryPoolPinBadOffsetTooLarge()
123123
{
124124
MemoryPool<int> pool = MemoryPool<int>.Shared;
125-
IMemoryOwner<int> block = pool.Rent(10);
125+
using IMemoryOwner<int> block = pool.Rent(10);
126126
Memory<int> memory = block.Memory;
127127
Span<int> sp = memory.Span;
128128
Assert.Equal(memory.Length, sp.Length);

src/libraries/System.Net.Http.Json/tests/UnitTests/TranscodingWriteStreamTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private static async Task WriteAsyncTest(Encoding targetEncoding, string message
6767
var model = new TestModel { Message = message };
6868
var stream = new MemoryStream();
6969

70-
var transcodingStream = new TranscodingWriteStream(stream, targetEncoding);
70+
using var transcodingStream = new TranscodingWriteStream(stream, targetEncoding);
7171
await JsonSerializer.SerializeAsync(transcodingStream, model, model.GetType());
7272
// The transcoding streams use Encoders and Decoders that have internal buffers. We need to flush these
7373
// when there is no more data to be written. Stream.FlushAsync isn't suitable since it's

src/libraries/System.Net.Http/tests/FunctionalTests/NtAuthTests.FakeServer.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ internal static async Task HandleAuthenticationRequestWithFakeServer(LoopbackSer
107107
}
108108
while (!isAuthenticated);
109109

110+
fakeNtlmServer?.Dispose();
111+
110112
await connection.SendResponseAsync(HttpStatusCode.OK);
111113
}
112114

0 commit comments

Comments
 (0)