diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 7024eb6c548da..f6508b0b8a26e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -4,7 +4,6 @@ using Microsoft.Win32.SafeHandles; using System.Collections.Generic; using System.ComponentModel; -using System.Globalization; using System.IO; using System.IO.Pipes; using System.Security; @@ -760,7 +759,7 @@ internal static TimeSpan TicksToTimeSpan(double ticks) private static Stream OpenStream(int fd, PipeDirection direction) { Debug.Assert(fd >= 0); - return new AnonymousPipeClientStream(direction, fd.ToString(CultureInfo.InvariantCulture)); + return new AnonymousPipeClientStream(direction, new SafePipeHandle((IntPtr)fd, ownsHandle: true)); } /// Parses a command-line argument string into a list of arguments. diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs index 577a9f1a06137..39e068d409dd6 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs @@ -344,7 +344,6 @@ async private Task WaitPipeSignal(PipeStream pipe, int millisecond) } } - [ActiveIssue("https://github.com/dotnet/runtime/issues/44329")] [PlatformSpecific(~TestPlatforms.Windows)] // currently on Windows these operations async-over-sync on Windows [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public async Task ReadAsync_OutputStreams_Cancel_RespondsQuickly() diff --git a/src/libraries/System.IO.Pipes/src/Microsoft/Win32/SafeHandles/SafePipeHandle.Unix.cs b/src/libraries/System.IO.Pipes/src/Microsoft/Win32/SafeHandles/SafePipeHandle.Unix.cs index ea4c98560cb11..e52957f40aa69 100644 --- a/src/libraries/System.IO.Pipes/src/Microsoft/Win32/SafeHandles/SafePipeHandle.Unix.cs +++ b/src/libraries/System.IO.Pipes/src/Microsoft/Win32/SafeHandles/SafePipeHandle.Unix.cs @@ -40,8 +40,7 @@ protected override void Dispose(bool disposing) { base.Dispose(disposing); // must be called before trying to Dispose the socket _disposed = 1; - Socket? socket; - if (disposing && (socket = Volatile.Read(ref _pipeSocket)) != null) + if (disposing && Volatile.Read(ref _pipeSocket) is Socket socket) { socket.Dispose(); _pipeSocket = null; @@ -84,6 +83,8 @@ private Socket CreatePipeSocket(bool ownsHandle = true) socket = SetPipeSocketInterlocked(new Socket(new SafeSocketHandle(handle, ownsHandle)), ownsHandle); + // Double check if we haven't Disposed in the meanwhile, and ensure + // the Socket is disposed, in case Dispose() missed the _pipeSocket assignment. if (_disposed == 1) { Volatile.Write(ref _pipeSocket, null); diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs index 852b25f9c0f3e..1cef1b1d39f46 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs @@ -244,60 +244,58 @@ private unsafe SocketError DoCloseHandle(bool abortive) { return SocketPal.GetSocketErrorForErrorCode(CloseHandle(handle)); } - else - { - // If abortive is not set, we're not running on the finalizer thread, so it's safe to block here. - // We can honor the linger options set on the socket. It also means closesocket() might return - // EWOULDBLOCK, in which case we need to do some recovery. - if (!abortive) - { - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"handle:{handle} Following 'non-abortive' branch."); - - // Close, and if its errno is other than EWOULDBLOCK, there's nothing more to do - we either succeeded or failed. - errorCode = CloseHandle(handle); - if (errorCode != Interop.Error.EWOULDBLOCK) - { - return SocketPal.GetSocketErrorForErrorCode(errorCode); - } - // The socket must be non-blocking with a linger timeout set. - // We have to set the socket to blocking. - if (Interop.Sys.Fcntl.DangerousSetIsNonBlocking(handle, 0) == 0) - { - // The socket successfully made blocking; retry the close(). - return SocketPal.GetSocketErrorForErrorCode(CloseHandle(handle)); - } + // If abortive is not set, we're not running on the finalizer thread, so it's safe to block here. + // We can honor the linger options set on the socket. It also means closesocket() might return + // EWOULDBLOCK, in which case we need to do some recovery. + if (!abortive) + { + if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"handle:{handle} Following 'non-abortive' branch."); - // The socket could not be made blocking; fall through to the regular abortive close. + // Close, and if its errno is other than EWOULDBLOCK, there's nothing more to do - we either succeeded or failed. + errorCode = CloseHandle(handle); + if (errorCode != Interop.Error.EWOULDBLOCK) + { + return SocketPal.GetSocketErrorForErrorCode(errorCode); } - // By default or if the non-abortive path failed, set linger timeout to zero to get an abortive close (RST). - var linger = new Interop.Sys.LingerOption + // The socket must be non-blocking with a linger timeout set. + // We have to set the socket to blocking. + if (Interop.Sys.Fcntl.DangerousSetIsNonBlocking(handle, 0) == 0) { - OnOff = 1, - Seconds = 0 - }; + // The socket successfully made blocking; retry the close(). + return SocketPal.GetSocketErrorForErrorCode(CloseHandle(handle)); + } - errorCode = Interop.Sys.SetLingerOption(handle, &linger); - #if DEBUG - _closeSocketLinger = SocketPal.GetSocketErrorForErrorCode(errorCode); - #endif - if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"handle:{handle}, setsockopt():{errorCode}"); + // The socket could not be made blocking; fall through to the regular abortive close. + } - switch (errorCode) - { - case Interop.Error.SUCCESS: - case Interop.Error.EINVAL: - case Interop.Error.ENOPROTOOPT: - case Interop.Error.ENOTSOCK: - errorCode = CloseHandle(handle); - break; - - // For other errors, it's too dangerous to try closesocket() - it might block! - } + // By default or if the non-abortive path failed, set linger timeout to zero to get an abortive close (RST). + var linger = new Interop.Sys.LingerOption + { + OnOff = 1, + Seconds = 0 + }; + + errorCode = Interop.Sys.SetLingerOption(handle, &linger); +#if DEBUG + _closeSocketLinger = SocketPal.GetSocketErrorForErrorCode(errorCode); +#endif + if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"handle:{handle}, setsockopt():{errorCode}"); + + switch (errorCode) + { + case Interop.Error.SUCCESS: + case Interop.Error.EINVAL: + case Interop.Error.ENOPROTOOPT: + case Interop.Error.ENOTSOCK: + errorCode = CloseHandle(handle); + break; - return SocketPal.GetSocketErrorForErrorCode(errorCode); + // For other errors, it's too dangerous to try closesocket() - it might block! } + + return SocketPal.GetSocketErrorForErrorCode(errorCode); } private Interop.Error CloseHandle(IntPtr handle) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs index 61c0783da9bc7..bf57c9311602c 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs @@ -787,15 +787,12 @@ public bool StartAsyncOperation(SocketAsyncContext context, TOperation operation { Trace(context, $"Enter"); - if (!context.IsRegistered) + if (!context.IsRegistered && !context.TryRegister(out Interop.Error error)) { - if (!context.TryRegister(out Interop.Error error)) - { - HandleFailedRegistration(context, operation, error); + HandleFailedRegistration(context, operation, error); - Trace(context, $"Leave, not registered"); - return false; - } + Trace(context, "Leave, not registered"); + return false; } while (true) @@ -881,7 +878,8 @@ static void HandleFailedRegistration(SocketAsyncContext context, TOperation oper // macOS: kevent returns EPIPE when adding pipe fd for which the other end is closed. if (error == Interop.Error.EPIPE) { - // Retry the operation. + // Because the other end close, we expect the operation to complete when we retry it. + // If it doesn't, we fall through and throw an Exception. if (operation.TryComplete(context)) { return; diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs index 20c421440ae39..3c1b8863ea5bf 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs @@ -177,7 +177,7 @@ private static unsafe int SysWrite(SafeSocketHandle socket, ReadOnlySpan b fixed (byte* b = &MemoryMarshal.GetReference(buffer)) { - sent = Interop.Sys.Write(socket, &b[offset], buffer.Length); + sent = Interop.Sys.Write(socket, b + offset, count); if (sent == -1) { errno = Interop.Sys.GetLastError(); @@ -202,7 +202,7 @@ private static unsafe int SysSend(SafeSocketHandle socket, SocketFlags flags, Re { errno = Interop.Sys.Send( socket, - &b[offset], + b + offset, count, flags, &sent); @@ -228,7 +228,7 @@ private static unsafe int SysSend(SafeSocketHandle socket, SocketFlags flags, Re { var iov = new Interop.Sys.IOVector { - Base = &b[offset], + Base = b + offset, Count = (UIntPtr)count };