- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
Closed
Closed
Copy link
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-System.IOhelp wanted[up-for-grabs] Good issue for external contributors[up-for-grabs] Good issue for external contributors
Milestone
Description
Background and motivation
As described by #32760 (comment), the constructor
public NamedPipeClientStream(System.IO.Pipes.PipeDirection direction, bool isAsync, bool isConnected, Microsoft.Win32.SafeHandles.SafePipeHandle safePipeHandle)Using isConnected: false is invalid, it will throw an exception when Connect is called because it requires a path to establish the connection.  We should discourage the use of this ctor and instead suggest using a new ctor that doesn't take bool isConnected.
To preserve compatibility in the existing ctor, I think we should ignore isConnected rather than throwing.
API Proposal
namespace System.IO.Pipes;
public sealed partial class NamedPipeClientStream : PipeStream
{
+   [EditorBrowsable(EditorBrowsableState.Never)]
+   [Obsolete("This constructor has been deprecated and argument bool isConnected does not have any effect. Use NamedPipeClientStream(PipeDirection direction, bool isAsync, SafePipeHandle safePipeHandle) instead.")]
    public NamedPipeClientStream(PipeDirection direction, bool isAsync, bool isConnected, SafePipeHandle safePipeHandle) : base (default(PipeDirection), default(int)) { }
+   public NamedPipeClientStream(PipeDirection direction, bool isAsync, SafePipeHandle safePipeHandle) : base(default(PipeDirection), default(int)) { }
}Alternative Designs
None that I can think of, but I considered fixing the issue by using the provided handle to reconnect to the named pipe server. That doesn't seem feasible, A client Connects by obtaining a handle (via CreateFile in Windows and via Socket.Connect on Unix). reconnecting would imply discarding the existing handle.
Risks
No response
Copilot
Metadata
Metadata
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-System.IOhelp wanted[up-for-grabs] Good issue for external contributors[up-for-grabs] Good issue for external contributors