Skip to content

Commit 0fffb89

Browse files
authored
Anonymous pipe resource leak when DisposeLocalCopyOfClientHandle is not called (#78562)
1 parent 722745e commit 0fffb89

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Diagnostics.CodeAnalysis;
54
using Microsoft.Win32.SafeHandles;
65

76
namespace System.IO.Pipes
@@ -12,7 +11,8 @@ namespace System.IO.Pipes
1211
public sealed partial class AnonymousPipeServerStream : PipeStream
1312
{
1413
private SafePipeHandle _clientHandle = null!;
15-
private bool _clientHandleExposed;
14+
private bool _clientHandleExposed, _clientHandleExposedAsString;
15+
private readonly HandleInheritability _inheritability;
1616

1717
public AnonymousPipeServerStream()
1818
: this(PipeDirection.Out, HandleInheritability.None, 0)
@@ -73,6 +73,7 @@ public AnonymousPipeServerStream(PipeDirection direction, HandleInheritability i
7373
}
7474

7575
Create(direction, inheritability, bufferSize);
76+
_inheritability = inheritability;
7677
}
7778

7879
~AnonymousPipeServerStream()
@@ -84,7 +85,7 @@ public AnonymousPipeServerStream(PipeDirection direction, HandleInheritability i
8485
// processes. For now, people do it via command line arguments.
8586
public string GetClientHandleAsString()
8687
{
87-
_clientHandleExposed = true;
88+
_clientHandleExposedAsString =_clientHandleExposed = true;
8889
GC.SuppressFinalize(_clientHandle);
8990
return _clientHandle.DangerousGetHandle().ToString();
9091
}
@@ -121,10 +122,11 @@ protected override void Dispose(bool disposing)
121122
{
122123
try
123124
{
124-
// We should dispose of the client handle if it was not exposed.
125-
if (!_clientHandleExposed && _clientHandle != null && !_clientHandle.IsClosed)
125+
// We should dispose of the client handle when it was not exposed at all OR
126+
// it was exposed as a string (handle finalization has been suppressed) and created inheritable (out-of-proc communication).
127+
if (!_clientHandleExposed || (_clientHandleExposedAsString && _inheritability == HandleInheritability.Inheritable))
126128
{
127-
_clientHandle.Dispose();
129+
DisposeLocalCopyOfClientHandle();
128130
}
129131
}
130132
finally

src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CrossProcess.cs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Diagnostics;
54
using System.Threading;
65
using Microsoft.DotNet.RemoteExecutor;
76
using Xunit;
@@ -48,16 +47,23 @@ void ChildFunc(string inHandle, string outHandle)
4847
}
4948
}
5049

51-
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
52-
public void ServerClosesPipe_ClientReceivesEof()
50+
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
51+
[InlineData(true)]
52+
[InlineData(false)]
53+
public void ServerClosesPipe_ClientReceivesEof(bool callDisposeLocalCopyOfClientHandle)
5354
{
5455
using (var pipe = new AnonymousPipeServerStream(PipeDirection.Out, HandleInheritability.Inheritable))
5556
using (var remote = RemoteExecutor.Invoke(new Action<string>(ChildFunc), pipe.GetClientHandleAsString()))
5657
{
57-
pipe.DisposeLocalCopyOfClientHandle();
58+
if (callDisposeLocalCopyOfClientHandle)
59+
{
60+
pipe.DisposeLocalCopyOfClientHandle();
61+
}
62+
5863
pipe.Write(new byte[] { 1, 2, 3, 4, 5 }, 0, 5);
5964

6065
pipe.Dispose();
66+
Assert.True(pipe.ClientSafePipeHandle.IsClosed);
6167

6268
Assert.True(remote.Process.WaitForExit(30_000));
6369
}

0 commit comments

Comments
 (0)