Skip to content

Commit cc92235

Browse files
committed
Update after review
1 parent a2333c4 commit cc92235

File tree

3 files changed

+11
-11
lines changed

3 files changed

+11
-11
lines changed

Documentation/file-descriptors.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ Conceptually, the relationship between `FD` (a number) and `StreamBox` (a class)
1515

1616
It is possible to have the structure above without `FileIO`; for instance when an OS file is opened with one of the low-level functions in `os`, or when an existing FD is duplicated. It is also possible to associate an FD with several `FileIO`. In such cases it is the responsibility of the user code to take care that the FD is closed at the right time.
1717

18-
When FD is duplicated (using `dup` or `dup2`), the associated `StreamBox` is duplicated (there is always a 1-to1 relationship between FD and `StreamBox`), but the underlying `FileStream` objects remain the same, and so are the underlying OS handles. The new FD may be used to create a `FileIO` (or several, just as the original FD). All read/seek/write operations on both descriptors go though the same `FileStream` object and the same OS handle.
18+
When FD is duplicated (using `dup` or `dup2`), the associated `StreamBox` is duplicated (there is always a 1-to-1 relationship between FD and `StreamBox`), but the underlying `FileStream` objects remain the same, and so are the underlying OS handles. The new FD may be used to create a `FileIO` (or several, just as the original FD). All read/seek/write operations on both descriptors go though the same `FileStream` object and the same OS handle.
1919

2020
```mermaid
2121
graph LR;
@@ -52,7 +52,7 @@ FD2(FD2) <--> StreamBox2[StreamBox] --> FileStream2[FileStream] --> FDH2(FD2) --
5252

5353
In this way, the file descriptor on the `PythonFileManager` level is the same as the file descriptor used by `FileStream`.
5454

55-
Unfortunately, on .NET, somehow, the two independent read/write positions. This is not how duplicated file descriptors should work: both descriptors should point to the same file description structure and share the read/seek/write position. In practice, on .NET, writing through the second file object will overwrite data already written through the first file object. In regular Unix applications (incl. CPython), the subsequent writes append data, regardless which file object is used. The same principle should apply to reads.
55+
Unfortunately, on .NET, somehow, two `FileStream` instances using the same file descriptor will have the two independent read/write positions. This is not how duplicated file descriptors should work: both descriptors should point to the same file description structure and share the read/seek/write position. In practice, on .NET, writing through the second file object will overwrite data already written through the first file object. In regular Unix applications (incl. CPython), the subsequent writes append data, regardless which file object is used. The same principle should apply to reads.
5656

5757
Also unfortunately, on Mono, the `FileStream` constructor accepts only descriptors opened by another call to a `FileStream` constructor[[1]]. So descriptors obtained from direct OS calls, like `open`, `creat`, `dup`, `dup2` are being rejected.
5858

Src/IronPython.Modules/nt.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -351,11 +351,7 @@ public static int dup(CodeContext/*!*/ context, int fd) {
351351
PythonFileManager fileManager = context.LanguageContext.FileManager;
352352

353353
StreamBox streams = fileManager.GetStreams(fd); // OSError if fd not valid
354-
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
355-
fileManager.EnsureRefStreams(streams);
356-
fileManager.AddRefStreams(streams);
357-
return fileManager.Add(new(streams));
358-
} else {
354+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
359355
if (!streams.IsSingleStream && fd is 1 or 2) {
360356
// If there is a separate write stream, dupping over stout or sderr uses write stream's file descriptor
361357
fd = streams.WriteStream is FileStream fs ? fs.SafeFileHandle.DangerousGetHandle().ToInt32() : fd;
@@ -369,6 +365,10 @@ public static int dup(CodeContext/*!*/ context, int fd) {
369365
fileManager.AddRefStreams(streams);
370366
return fileManager.Add(fd2, new(streams));
371367
}
368+
} else {
369+
fileManager.EnsureRefStreams(streams);
370+
fileManager.AddRefStreams(streams);
371+
return fileManager.Add(new(streams));
372372
}
373373
}
374374

@@ -389,7 +389,7 @@ public static int dup2(CodeContext/*!*/ context, int fd, int fd2) {
389389
close(context, fd2);
390390
}
391391

392-
// TODO: race condition: `open` or `dup` on another thread may occupy fd2 (simulated desctiptors only)
392+
// TODO: race condition: `open` or `dup` on another thread may occupy fd2 (simulated descriptors only)
393393

394394
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
395395
fileManager.EnsureRefStreams(streams);
@@ -418,7 +418,7 @@ private static int UnixDup(int fd, int fd2, out Stream? stream) {
418418
int res = fd2 < 0 ? Mono.Unix.Native.Syscall.dup(fd) : Mono.Unix.Native.Syscall.dup2(fd, fd2);
419419
if (res < 0) throw GetLastUnixError();
420420
if (ClrModule.IsMono) {
421-
// This does not work on .NET, probably because .NET FileStream is not aware of Mono.Unix.UnxiStream
421+
// This does not work on .NET, probably because .NET FileStream is not aware of Mono.Unix.UnixStream
422422
stream = new Mono.Unix.UnixStream(res, ownsHandle: true);
423423
} else {
424424
// This does not work 100% correctly on .NET, probably because each FileStream has its own read/write cursor

Src/IronPython/Runtime/PythonFileManager.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,8 @@ public int Add(int id, StreamBox streams) {
234234
}
235235

236236
// This version of Add is used for emulated file descriptors.
237-
// Must not be used on Unix.
238-
[SupportedOSPlatform("windows")]
237+
// Must not be used on POSIX.
238+
[UnsupportedOSPlatform("linux"), UnsupportedOSPlatform("macos")]
239239
public int Add(StreamBox streams) {
240240
ContractUtils.RequiresNotNull(streams, nameof(streams));
241241
ContractUtils.Requires(streams.Id < 0, nameof(streams));

0 commit comments

Comments
 (0)