From 8198b99935f562ffb2fb9a75175a8df24d235152 Mon Sep 17 00:00:00 2001 From: jhorv <38920027+jhorv@users.noreply.github.com> Date: Thu, 30 Mar 2023 16:07:07 -0400 Subject: [PATCH] Fix Linux hang on shutdown (#4617) * Rework StdErr-to-log redirection to use built-in FileStream, and do reads asynchronously to avoid hanging the process shutdown. * set _disposable to false ASAP --- Ryujinx.Common/SystemInterop/StdErrAdapter.cs | 50 ++++-- Ryujinx.Common/SystemInterop/UnixStream.cs | 155 ------------------ 2 files changed, 33 insertions(+), 172 deletions(-) delete mode 100644 Ryujinx.Common/SystemInterop/UnixStream.cs diff --git a/Ryujinx.Common/SystemInterop/StdErrAdapter.cs b/Ryujinx.Common/SystemInterop/StdErrAdapter.cs index efb1421843..b1ed7b6893 100644 --- a/Ryujinx.Common/SystemInterop/StdErrAdapter.cs +++ b/Ryujinx.Common/SystemInterop/StdErrAdapter.cs @@ -1,18 +1,21 @@ +using Microsoft.Win32.SafeHandles; using Ryujinx.Common.Logging; using System; using System.IO; using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Threading; +using System.Threading.Tasks; namespace Ryujinx.Common.SystemInterop { public partial class StdErrAdapter : IDisposable { private bool _disposable = false; - private UnixStream _pipeReader; - private UnixStream _pipeWriter; - private Thread _worker; + private Stream _pipeReader; + private Stream _pipeWriter; + private CancellationTokenSource _cancellationTokenSource; + private Task _worker; public StdErrAdapter() { @@ -31,37 +34,39 @@ namespace Ryujinx.Common.SystemInterop (int readFd, int writeFd) = MakePipe(); dup2(writeFd, stdErrFileno); - _pipeReader = new UnixStream(readFd); - _pipeWriter = new UnixStream(writeFd); + _pipeReader = CreateFileDescriptorStream(readFd); + _pipeWriter = CreateFileDescriptorStream(writeFd); - _worker = new Thread(EventWorker); + _cancellationTokenSource = new CancellationTokenSource(); + _worker = Task.Run(async () => await EventWorkerAsync(_cancellationTokenSource.Token), _cancellationTokenSource.Token); _disposable = true; - _worker.Start(); } - + [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] - private void EventWorker() + private async Task EventWorkerAsync(CancellationToken cancellationToken) { - TextReader reader = new StreamReader(_pipeReader); + using TextReader reader = new StreamReader(_pipeReader, leaveOpen: true); string line; - while ((line = reader.ReadLine()) != null) + while (cancellationToken.IsCancellationRequested == false && (line = await reader.ReadLineAsync(cancellationToken)) != null) { Logger.Error?.PrintRawMsg(line); } } - + private void Dispose(bool disposing) { if (_disposable) { + _disposable = false; + if (OperatingSystem.IsLinux() || OperatingSystem.IsMacOS()) { + _cancellationTokenSource.Cancel(); + _worker.Wait(0); _pipeReader?.Close(); _pipeWriter?.Close(); } - - _disposable = false; } } @@ -74,11 +79,11 @@ namespace Ryujinx.Common.SystemInterop private static partial int dup2(int fd, int fd2); [LibraryImport("libc", SetLastError = true)] - private static unsafe partial int pipe(int* pipefd); + private static partial int pipe(Span pipefd); - private static unsafe (int, int) MakePipe() + private static (int, int) MakePipe() { - int *pipefd = stackalloc int[2]; + Span pipefd = stackalloc int[2]; if (pipe(pipefd) == 0) { @@ -89,5 +94,16 @@ namespace Ryujinx.Common.SystemInterop throw new(); } } + + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] + private static Stream CreateFileDescriptorStream(int fd) + { + return new FileStream( + new SafeFileHandle((IntPtr)fd, ownsHandle: true), + FileAccess.ReadWrite + ); + } + } } diff --git a/Ryujinx.Common/SystemInterop/UnixStream.cs b/Ryujinx.Common/SystemInterop/UnixStream.cs deleted file mode 100644 index 1d64499743..0000000000 --- a/Ryujinx.Common/SystemInterop/UnixStream.cs +++ /dev/null @@ -1,155 +0,0 @@ -using System; -using System.IO; -using System.Runtime.InteropServices; -using System.Runtime.Versioning; - -namespace Ryujinx.Common.SystemInterop -{ - [SupportedOSPlatform("linux")] - [SupportedOSPlatform("macos")] - public partial class UnixStream : Stream, IDisposable - { - private const int InvalidFd = -1; - - private int _fd; - - [LibraryImport("libc", SetLastError = true)] - private static partial long read(int fd, IntPtr buf, ulong count); - - [LibraryImport("libc", SetLastError = true)] - private static partial long write(int fd, IntPtr buf, ulong count); - - [LibraryImport("libc", SetLastError = true)] - private static partial int close(int fd); - - public UnixStream(int fd) - { - if (InvalidFd == fd) - { - throw new ArgumentException("Invalid file descriptor"); - } - - _fd = fd; - - CanRead = read(fd, IntPtr.Zero, 0) != -1; - CanWrite = write(fd, IntPtr.Zero, 0) != -1; - } - - ~UnixStream() - { - Close(); - } - - public override bool CanRead { get; } - public override bool CanWrite { get; } - public override bool CanSeek => false; - - public override long Length => throw new NotSupportedException(); - - public override long Position - { - get => throw new NotSupportedException(); - set => throw new NotSupportedException(); - } - - public override void Flush() - { - } - - public override unsafe int Read([In, Out] byte[] buffer, int offset, int count) - { - if (offset < 0 || offset > (buffer.Length - count) || count < 0) - { - throw new ArgumentOutOfRangeException(); - } - - if (buffer.Length == 0) - { - return 0; - } - - long r = 0; - fixed (byte* buf = &buffer[offset]) - { - do - { - r = read(_fd, (IntPtr)buf, (ulong)count); - } while (ShouldRetry(r)); - } - - return (int)r; - } - - public override unsafe void Write(byte[] buffer, int offset, int count) - { - if (offset < 0 || offset > (buffer.Length - count) || count < 0) - { - throw new ArgumentOutOfRangeException(); - } - - if (buffer.Length == 0) - { - return; - } - - fixed (byte* buf = &buffer[offset]) - { - long r = 0; - do { - r = write(_fd, (IntPtr)buf, (ulong)count); - } while (ShouldRetry(r)); - } - } - - public override long Seek(long offset, SeekOrigin origin) - { - throw new NotSupportedException(); - } - - public override void SetLength(long value) - { - throw new NotSupportedException(); - } - - public override void Close() - { - if (_fd == InvalidFd) - { - return; - } - - Flush(); - - int r; - do { - r = close(_fd); - } while (ShouldRetry(r)); - - _fd = InvalidFd; - } - - void IDisposable.Dispose() - { - Close(); - } - - private bool ShouldRetry(long r) - { - if (r == -1) - { - const int eintr = 4; - - int errno = Marshal.GetLastPInvokeError(); - - if (errno == eintr) - { - return true; - } - - throw new SystemException($"Operation failed with error 0x{errno:X}"); - } - - return false; - } - } -}