Skip to content

Commit

Permalink
Maintain file position when accessing file descriptor
Browse files Browse the repository at this point in the history
  • Loading branch information
@BCSharp
BCSharp committed Dec 12, 2024
1 parent c1edb43 commit a1e894e
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 19 deletions.
34 changes: 20 additions & 14 deletions Src/IronPython.Modules/nt.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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));
}
Expand Down
13 changes: 13 additions & 0 deletions Src/IronPython/Modules/_fileio.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions Src/IronPython/Runtime/PythonFileManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()),
Expand Down

0 comments on commit a1e894e

Please sign in to comment.