Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support mode ab+ equivalent through os.open #1839

Merged
merged 4 commits into from
Dec 13, 2024
Merged

Support mode ab+ equivalent through os.open #1839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1edb43
Use dual stream for os.open with O_APPEND
BCSharp Dec 11, 2024
a1e894e
Maintain file position when accessing file descriptor
BCSharp Dec 12, 2024
d51c7a5
Add test
BCSharp Dec 12, 2024
941e3c2
Fix typo
BCSharp Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions Src/IronPython.Modules/nt.cs
View file
Open in desktop
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 @@ -877,8 +875,9 @@ public static object open(CodeContext/*!*/ context, [NotNone] string path, int f
FileMode fileMode = FileModeFromFlags(flags);
FileAccess access = FileAccessFromFlags(flags);
FileOptions options = FileOptionsFromFlags(flags);
Stream s;
FileStream? fs;
Stream s; // the stream opened to acces the file
FileStream? fs; // downcast of s if s is FileStream (this is always the case on POSIX)
Stream? rs = null; // secondary read stream if needed, otherwise same as s
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && IsNulFile(path)) {
fs = null;
s = Stream.Null;
Expand All @@ -889,15 +888,28 @@ public static object open(CodeContext/*!*/ context, [NotNone] string path, int f
fs.Close();
s = fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, DefaultBufferSize, options);
} else if (access == FileAccess.ReadWrite && fileMode == FileMode.Append) {
// .NET doesn't allow Append w/ access != Write, so open the file w/ Write
// and a secondary stream w/ Read, then seek to the end.
s = fs = new FileStream(path, FileMode.Append, FileAccess.Write, FileShare.ReadWrite, DefaultBufferSize, options);
rs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, DefaultBufferSize, options);
rs.Seek(0L, SeekOrigin.End);
} else {
s = fs = new FileStream(path, fileMode, access, FileShare.ReadWrite, DefaultBufferSize, options);
}
rs ??= s;

if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
return context.LanguageContext.FileManager.Add(new(s));
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
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((int)fs!.SafeFileHandle.DangerousGetHandle(), new(s));
return context.LanguageContext.FileManager.Add(new(rs, s));
}
} catch (Exception e) {
throw ToPythonException(e, path);
Expand Down
13 changes: 13 additions & 0 deletions Src/IronPython/Modules/_fileio.cs
View file
Open in desktop
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);
// according 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
Open in desktop
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
24 changes: 24 additions & 0 deletions Tests/modules/system_related/test_os.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@
from iptest import IronPythonTestCase, is_osx, is_linux, is_windows, run_test

class OsTest(IronPythonTestCase):
def setUp(self):
super(OsTest, self).setUp()
self.temp_file = os.path.join(self.temporary_dir, "temp_OSTest_%d.dat" % os.getpid())

def tearDown(self):
self.delete_files(self.temp_file)
return super().tearDown()

def test_strerror(self):
if is_windows:
self.assertEqual(os.strerror(0), "No error")
Expand All @@ -29,4 +37,20 @@ def test_strerror(self):
elif is_osx:
self.assertEqual(os.strerror(40), "Message too long")

def test_open_abplus(self):
# equivalent to open(self.temp_file, "ab+"), see also test_file.test_open_abplus
fd = os.open(self.temp_file, os.O_APPEND | os.O_CREAT | os.O_RDWR)
try:
f = open(fd, mode="ab+", closefd=False)
f.write(b"abc")
f.seek(0)
self.assertEqual(f.read(2), b"ab")
f.write(b"def")
self.assertEqual(f.read(2), b"")
f.seek(0)
self.assertEqual(f.read(6), b"abcdef")
f.close()
finally:
os.close(fd)

run_test(__name__)
Loading