From a1e894e87afac6e6f1f31c1e7521e98af72a0440 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Wed, 11 Dec 2024 19:38:03 -0800 Subject: [PATCH] Maintain file position when accessing file descriptor --- Src/IronPython.Modules/nt.cs | 34 ++++++++++++--------- Src/IronPython/Modules/_fileio.cs | 13 ++++++++ Src/IronPython/Runtime/PythonFileManager.cs | 10 +++--- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/Src/IronPython.Modules/nt.cs b/Src/IronPython.Modules/nt.cs index c5ac5b3c1..5ef569320 100644 --- a/Src/IronPython.Modules/nt.cs +++ b/Src/IronPython.Modules/nt.cs @@ -352,10 +352,6 @@ public static int dup(CodeContext/*!*/ context, int fd) { StreamBox streams = fileManager.GetStreams(fd); // OSError if fd not valid if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - if (!streams.IsSingleStream && fd is 1 or 2) { - // If there is a separate write stream, dupping over stout or sderr uses write stream's file descriptor - fd = streams.WriteStream is FileStream fs ? fs.SafeFileHandle.DangerousGetHandle().ToInt32() : fd; - } int fd2 = UnixDup(fd, -1, out Stream? dupstream); if (dupstream is not null) { return fileManager.Add(fd2, new(dupstream)); @@ -389,16 +385,14 @@ public static int dup2(CodeContext/*!*/ context, int fd, int fd2) { close(context, fd2); } - // TODO: race condition: `open` or `dup` on another thread may occupy fd2 (simulated descriptors only) + // TODO: race condition: `open` or `dup` on another thread may occupy fd2 - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - fileManager.EnsureRefStreams(streams); - fileManager.AddRefStreams(streams); - return fileManager.Add(fd2, new(streams)); - } else { - if (!streams.IsSingleStream && fd is 1 or 2) { - // If there is a separate write stream, dupping over stout or sderr uses write stream's file descriptor - fd = streams.WriteStream is FileStream fs ? fs.SafeFileHandle.DangerousGetHandle().ToInt32() : fd; + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { + if (!streams.IsSingleStream && fd2 is 0 && streams.ReadStream is FileStream fs) { + // If there is a separate read stream, dupping over stdin uses read stream's file descriptor as source + long pos = fs.Position; + fd = fs.SafeFileHandle.DangerousGetHandle().ToInt32(); + fs.Seek(pos, SeekOrigin.Begin); } fd2 = UnixDup(fd, fd2, out Stream? dupstream); // closes fd2 atomically if reopened in the meantime fileManager.Remove(fd2); @@ -410,6 +404,10 @@ public static int dup2(CodeContext/*!*/ context, int fd, int fd2) { fileManager.AddRefStreams(streams); return fileManager.Add(fd2, new(streams)); } + } else { + fileManager.EnsureRefStreams(streams); + fileManager.AddRefStreams(streams); + return fileManager.Add(fd2, new(streams)); } } @@ -901,7 +899,15 @@ public static object open(CodeContext/*!*/ context, [NotNone] string path, int f rs ??= s; if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - return context.LanguageContext.FileManager.Add(fs!.SafeFileHandle.DangerousGetHandle().ToInt32(), new(rs, s)); + int fd = fs!.SafeFileHandle.DangerousGetHandle().ToInt32(); + // accessing SafeFileHandle may reset file position + if (fileMode == FileMode.Append) { + fs.Seek(0L, SeekOrigin.End); + } + if (!ReferenceEquals(fs, rs)) { + rs.Seek(fs.Position, SeekOrigin.Begin); + } + return context.LanguageContext.FileManager.Add(fd, new(rs, s)); } else { return context.LanguageContext.FileManager.Add(new(rs, s)); } diff --git a/Src/IronPython/Modules/_fileio.cs b/Src/IronPython/Modules/_fileio.cs index 93e2135bf..86ffa1c9c 100644 --- a/Src/IronPython/Modules/_fileio.cs +++ b/Src/IronPython/Modules/_fileio.cs @@ -127,6 +127,19 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo default: throw new InvalidOperationException(); } + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { + // On POSIX, register the file descriptor with the file manager right after file opening + _context.FileManager.GetOrAssignId(_streams); + // accoding to [documentation](https://learn.microsoft.com/en-us/dotnet/api/system.io.filestream.safefilehandle?view=net-9.0#remarks) + // accessing SafeFileHandle sets the current stream position to 0 + // in practice it doesn't seem to be the case, but better to be sure + if (this.mode == "ab+") { + _streams.WriteStream.Seek(0L, SeekOrigin.End); + } + if (!_streams.IsSingleStream) { + _streams.ReadStream.Seek(_streams.WriteStream.Position, SeekOrigin.Begin); + } + } } else { object fdobj = PythonOps.CallWithContext(context, opener, name, flags); diff --git a/Src/IronPython/Runtime/PythonFileManager.cs b/Src/IronPython/Runtime/PythonFileManager.cs index 64a81bb9b..c98811608 100644 --- a/Src/IronPython/Runtime/PythonFileManager.cs +++ b/Src/IronPython/Runtime/PythonFileManager.cs @@ -287,12 +287,12 @@ public int GetOrAssignId(StreamBox streams) { lock (_synchObject) { int res = streams.Id; if (res == -1) { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - res = Add(streams); - } else { - res = GetGenuineFileDescriptor(streams.ReadStream); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { + res = GetGenuineFileDescriptor(streams.WriteStream); if (res < 0) throw new InvalidOperationException("stream not associated with a file descriptor"); Add(res, streams); + } else { + res = Add(streams); } } return res; @@ -327,7 +327,7 @@ public bool ValidateFdRange(int fd) { return fd >= 0 && fd < LIMIT_OFILE; } - [UnsupportedOSPlatform("windows")] + [SupportedOSPlatform("linux"), SupportedOSPlatform("macos")] private static int GetGenuineFileDescriptor(Stream stream) { return stream switch { FileStream fs => checked((int)fs.SafeFileHandle.DangerousGetHandle()),