Maintain file position when accessing file descriptor

Author: BCSharp

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));
                 }

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);

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()),