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

Use PosixFileStream for files on POSIX #1855

Merged
merged 9 commits into from
Jan 9, 2025
Merged

Use PosixFileStream for files on POSIX #1855

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
86adef5
Use PosixFileStream for files on POSIX
BCSharp Dec 30, 2024
64d0d4a
Improve non-buffering strategy for FileStream
BCSharp Dec 31, 2024
1cf9ac6
Ignore OSException along with IOException
BCSharp Jan 1, 2025
847db52
Merge branch 'upstream/main' into 'open_file_posix'
BCSharp Jan 6, 2025
215237c
Fix file size checks in constructor
BCSharp Jan 6, 2025
f4cce8a
Implement proper lifetime management of handles
BCSharp Jan 7, 2025
a858fcd
Describe Rules of Engagement
BCSharp Jan 7, 2025
3bce846
Implement mmap.resize on POSIX
BCSharp Jan 7, 2025
91fc9f4
Update after review
BCSharp Jan 9, 2025
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
130 changes: 101 additions & 29 deletions Src/IronPython.Modules/mmap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System.Runtime.Serialization is VS trying to be helpful?

using System.Runtime.Versioning;
using System.Text;
using System.Threading;

Expand All @@ -24,6 +26,7 @@
using IronPython.Runtime.Types;

using Microsoft.Scripting.Utils;
using Microsoft.Win32.SafeHandles;

[assembly: PythonModule("mmap", typeof(IronPython.Modules.MmapModule))]
namespace IronPython.Modules {
Expand Down Expand Up @@ -92,6 +95,7 @@ public class MmapDefault : IWeakReferenceable {
private readonly long _offset;
private readonly string _mapName;
private readonly MemoryMappedFileAccess _fileAccess;
private readonly SafeFileHandle _handle; // only used on some POSIX platforms, null otherwise

private volatile bool _isClosed;
private int _refCount = 1;
Expand Down Expand Up @@ -148,46 +152,65 @@ public MmapDefault(CodeContext/*!*/ context, int fileno, long length, string tag

PythonContext pContext = context.LanguageContext;
if (pContext.FileManager.TryGetStreams(fileno, out StreamBox streams)) {
if ((_sourceStream = streams.ReadStream as FileStream) == null) {
throw WindowsError(PythonExceptions._OSError.ERROR_INVALID_HANDLE);
Stream stream = streams.ReadStream;
if (stream is FileStream fs) {
_sourceStream = fs;
} else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) {
// use file descriptor
#if NET8_0_OR_GREATER
CheckFileAccess(_fileAccess, stream);
_handle = new SafeFileHandle((IntPtr)fileno, ownsHandle: false);
_file = MemoryMappedFile.CreateFromFile(_handle, _mapName, length, _fileAccess, HandleInheritability.None, leaveOpen: true);
#else
_handle = new SafeFileHandle((IntPtr)fileno, ownsHandle: false);
FileAccess fileAccess = stream.CanWrite ? stream.CanRead ? FileAccess.ReadWrite : FileAccess.Write : FileAccess.Read;
// This may or may not work on Mono, but on Mono streams.ReadStream is FileStream (unless dupped in some cases)
_sourceStream = new FileStream(_handle, fileAccess);
#endif
}
// otherwise leaves _file as null and _sourceStream as null
} else {
throw PythonOps.OSError(PythonExceptions._OSError.ERROR_INVALID_BLOCK, "Bad file descriptor");
}

if (_fileAccess == MemoryMappedFileAccess.ReadWrite && !_sourceStream.CanWrite) {
throw WindowsError(PythonExceptions._OSError.ERROR_ACCESS_DENIED);
}
if (_file is null) {
// create _file form _sourceStream
if (_sourceStream is null) {
throw WindowsError(PythonExceptions._OSError.ERROR_INVALID_HANDLE);
}

CheckFileAccess(_fileAccess, _sourceStream);

if (length == 0) {
length = _sourceStream.Length;
if (length == 0) {
throw PythonOps.ValueError("cannot mmap an empty file");
}
if (_offset >= length) {
throw PythonOps.ValueError("mmap offset is greater than file size");
length = _sourceStream.Length;
if (length == 0) {
throw PythonOps.ValueError("cannot mmap an empty file");
}
if (_offset >= length) {
throw PythonOps.ValueError("mmap offset is greater than file size");
}
length -= _offset;
}
length -= _offset;
}

long capacity = checked(_offset + length);
long capacity = checked(_offset + length);

// Enlarge the file as needed.
if (capacity > _sourceStream.Length) {
if (_sourceStream.CanWrite) {
_sourceStream.SetLength(capacity);
} else {
throw WindowsError(PythonExceptions._OSError.ERROR_NOT_ENOUGH_MEMORY);
// Enlarge the file as needed.
if (capacity > _sourceStream.Length) {
if (_sourceStream.CanWrite) {
_sourceStream.SetLength(capacity);
} else {
throw WindowsError(PythonExceptions._OSError.ERROR_NOT_ENOUGH_MEMORY);
}
}
}

_file = CreateFromFile(
_sourceStream,
_mapName,
_sourceStream.Length,
_fileAccess,
HandleInheritability.None,
true);
_file = CreateFromFile(
_sourceStream,
_mapName,
_sourceStream.Length,
_fileAccess,
HandleInheritability.None,
true);
}
}

try {
Expand All @@ -198,7 +221,24 @@ public MmapDefault(CodeContext/*!*/ context, int fileno, long length, string tag
throw;
}
_position = 0L;
}

void CheckFileAccess(MemoryMappedFileAccess mmapAccess, Stream stream) {
bool isValid = mmapAccess switch {
MemoryMappedFileAccess.Read => stream.CanRead,
MemoryMappedFileAccess.ReadWrite => stream.CanRead && stream.CanWrite,
MemoryMappedFileAccess.CopyOnWrite => stream.CanRead,
_ => false
};

if (!isValid) {
if (_handle is not null && _sourceStream is not null) {
_sourceStream.Dispose();
}
throw PythonOps.OSError(PythonExceptions._OSError.ERROR_ACCESS_DENIED, "Invalid access mode");
}
}
} // end of constructor


public object __len__() {
using (new MmapLocker(this)) {
Expand Down Expand Up @@ -325,6 +365,11 @@ private void CloseWorker() {
_view.Flush();
_view.Dispose();
_file.Dispose();
if (_handle is not null) {
// mmap owns _sourceStream too in this case
_sourceStream?.Dispose();
_handle.Dispose();
}
_sourceStream = null;
_view = null;
_file = null;
Expand Down Expand Up @@ -557,6 +602,11 @@ public void resize(long newsize) {
}

if (_sourceStream == null) {
if (_handle is not null && !_handle.IsInvalid
&& (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Linux))) {
// resize on Posix platforms
PythonNT.ftruncateUnix(unchecked((int)_handle.DangerousGetHandle()), newsize);
}
// resizing is not supported without an underlying file
throw WindowsError(PythonExceptions._OSError.ERROR_INVALID_PARAMETER);
}
Expand Down Expand Up @@ -716,6 +766,9 @@ public void seek(long pos, int whence = SEEK_SET) {

public object size() {
using (new MmapLocker(this)) {
if (_handle is not null && (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) || RuntimeInformation.IsOSPlatform(OSPlatform.Linux))) {
return GetFileSizeUnix(_handle);
}
if (_sourceStream == null) return ReturnLong(_view.Capacity);
return ReturnLong(new FileInfo(_sourceStream.Name).Length);
}
Expand Down Expand Up @@ -830,6 +883,25 @@ internal Bytes GetSearchString() {
}
}

[SupportedOSPlatform("linux"), SupportedOSPlatform("macos")]
private static long GetFileSizeUnix(SafeFileHandle handle) {
long size;
if (handle.IsInvalid) {
throw PythonOps.OSError(PythonExceptions._OSError.ERROR_INVALID_HANDLE, "Invalid file handle");
}

if (Mono.Unix.Native.Syscall.fstat((int)handle.DangerousGetHandle(), out Mono.Unix.Native.Stat status) == 0) {
size = status.st_size;
} else {
Mono.Unix.Native.Errno errno = Mono.Unix.Native.Stdlib.GetLastError();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't use the PythonNT.GetLastUnixError here?

Copy link
Member Author

@BCSharp BCSharp Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PythonNT.GetLastUnixError cannot be used here (yet) because it does not have proper platform guards (yet). Adding the platform guards to PythonNT.GetLastUnixError has a cascading effect to the code. I am saving it for a separate PR (soon), not to mix up too much with this one.

string msg = Mono.Unix.UnixMarshal.GetErrorDescription(errno);
int error = Mono.Unix.Native.NativeConvert.FromErrno(errno);
throw PythonOps.OSError(error, msg);
}

return size;
}

#endregion

#region Synchronization
Expand Down
96 changes: 79 additions & 17 deletions Src/IronPython.Modules/nt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,22 +431,41 @@ public static int dup2(CodeContext/*!*/ context, int fd, int fd2) {
}


[SupportedOSPlatform("linux"), SupportedOSPlatform("osx")]
private static int UnixDup(int fd, int fd2, out Stream? stream) {
int res = fd2 < 0 ? Mono.Unix.Native.Syscall.dup(fd) : Mono.Unix.Native.Syscall.dup2(fd, fd2);
if (res < 0) throw GetLastUnixError();
if (ClrModule.IsMono) {
// This does not work on .NET, probably because .NET FileStream is not aware of Mono.Unix.UnixStream
stream = new Mono.Unix.UnixStream(res, ownsHandle: true);
// Elaborate workaround on Mono to avoid UnixStream as out
stream = new Mono.Unix.UnixStream(res, ownsHandle: false);
FileAccess fileAccess = stream.CanRead ? stream.CanWrite ? FileAccess.ReadWrite : FileAccess.Read : FileAccess.Write;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know the UnixStream details, but this seems to say that not being readable implies writable? Could there be odd cases where this isn't true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think it is possible. It's because there is actually no flag "readable". O_RDONLY is actually lack of any flags, i.e. 0b00. One can only request a writable file by setting flags 0b01 (non-readable writable) or 0b10 (readable writable). These two flags are mutually-exclusive, i.e. setting 0b11 results in EINVAL.

However, in other places I have the test inverted, i.e. stream.CanWrite ? stream.CanRead ?, which better reflects how those flags operate, so for consistency and clarity I'm going to change it here too. In other words, not being writable implies readable.

stream.Dispose();
try {
// FileStream on Mono created with a file descriptor might not work: /~https://github.com/mono/mono/issues/12783
// Test if it does, without closing the handle if it doesn't
var sfh = new SafeFileHandle((IntPtr)res, ownsHandle: false);
stream = new FileStream(sfh, fileAccess);
// No exception? Great! We can use FileStream.
stream.Dispose();
sfh.Dispose();
stream = null; // Create outside of try block
} catch (IOException) {
// Fall back to UnixStream
stream = new Mono.Unix.UnixStream(res, ownsHandle: true);
}
if (stream is null) {
// FileStream is safe
var sfh = new SafeFileHandle((IntPtr)res, ownsHandle: true);
stream = new FileStream(sfh, fileAccess);
}
} else {
// This does not work 100% correctly on .NET, probably because each FileStream has its own read/write cursor
// (it should be shared between dupped descriptors)
//stream = new FileStream(new SafeFileHandle((IntPtr)res, ownsHandle: true), FileAccess.ReadWrite);
// Accidentaly, this would also not work on Mono: /~https://github.com/mono/mono/issues/12783
stream = null; // Handle stream sharing in PythonFileManager
// normal case
stream = new PosixFileStream(res);
}
return res;
}


#if FEATURE_PROCESS
/// <summary>
/// single instance of environment dictionary is shared between multiple runtimes because the environment
Expand All @@ -470,6 +489,9 @@ public static object fstat(CodeContext/*!*/ context, int fd) {
PythonFileManager fileManager = context.LanguageContext.FileManager;

if (fileManager.TryGetStreams(fd, out StreamBox? streams)) {
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
return fstatUnix(fd);
}
if (streams.IsConsoleStream()) return new stat_result(0x2000);
if (streams.IsStandardIOStream()) return new stat_result(0x1000);
if (StatStream(streams.ReadStream) is not null and var res) return res;
Expand All @@ -483,15 +505,9 @@ public static object fstat(CodeContext/*!*/ context, int fd) {
#endif
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
if (ReferenceEquals(stream, Stream.Null)) return new stat_result(0x2000);
} else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
if (IsUnixStream(stream)) return new stat_result(0x1000);
}
return null;
}

static bool IsUnixStream(Stream stream) {
return stream is Mono.Unix.UnixStream;
}
}

public static void fsync(CodeContext context, int fd) {
Expand Down Expand Up @@ -869,7 +885,16 @@ public static void mkdir(CodeContext context, object? path, [ParamDictionary, No

private const int DefaultBufferSize = 4096;

[Documentation("open(path, flags, mode=511, *, dir_fd=None)")]
[Documentation("""
open(path, flags, mode=511, *, dir_fd=None)

Open a file for low level IO. Returns a file descriptor (integer).

If dir_fd is not None, it should be a file descriptor open to a directory,
and path should be relative; path will then be relative to that directory.
dir_fd may not be implemented on your platform.
If it is unavailable, using it will raise a NotImplementedError.
""")]
public static object open(CodeContext/*!*/ context, [NotNone] string path, int flags, [ParamDictionary, NotNone] IDictionary<string, object> kwargs, [NotNone] params object[] args) {
var numArgs = args.Length;
CheckOptionalArgsCount(numRegParms: 2, numOptPosParms: 1, numKwParms: 1, numArgs, kwargs.Count);
Expand All @@ -889,12 +914,23 @@ public static object open(CodeContext/*!*/ context, [NotNone] string path, int f
}
}

if ((RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) && !ClrModule.IsMono) {
// Use PosixFileStream to operate on fd directly
// On Mono, we must use FileStream due to limitations in MemoryMappedFile
Stream s = PosixFileStream.Open(path, flags, unchecked((uint)mode), out int fd);
//Stream s = PythonIOModule.FileIO.OpenFilePosix(path, flags, mode, out int fd);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the OpenFilePosix comment still relevant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I missed that one. I kept it around to switch back and forth between implementations when hunting for regressions.

if ((flags & O_APPEND) != 0) {
s.Seek(0L, SeekOrigin.End);
}
return context.LanguageContext.FileManager.Add(fd, new(s));
}

try {
FileMode fileMode = FileModeFromFlags(flags);
FileAccess access = FileAccessFromFlags(flags);
FileOptions options = FileOptionsFromFlags(flags);
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)
FileStream? fs; // downcast of s if s is FileStream
Stream? rs = null; // secondary read stream if needed, otherwise same as s
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && IsNulFile(path)) {
fs = null;
Expand Down Expand Up @@ -1436,6 +1472,13 @@ private static object statUnix(string path) {
return LightExceptions.Throw(GetLastUnixError(path));
}

private static object fstatUnix(int fd) {
if (Mono.Unix.Native.Syscall.fstat(fd, out Mono.Unix.Native.Stat buf) == 0) {
return new stat_result(buf);
}
return LightExceptions.Throw(GetLastUnixError());
}

private const int OPEN_EXISTING = 3;
private const int FILE_ATTRIBUTE_NORMAL = 0x00000080;
private const int FILE_READ_ATTRIBUTES = 0x0080;
Expand Down Expand Up @@ -1669,8 +1712,27 @@ public static void truncate(CodeContext context, object? path, BigInteger length
public static void truncate(CodeContext context, int fd, BigInteger length)
=> ftruncate(context, fd, length);

public static void ftruncate(CodeContext context, int fd, BigInteger length)
=> context.LanguageContext.FileManager.GetStreams(fd).Truncate((long)length);
public static void ftruncate(CodeContext context, int fd, BigInteger length) {
if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) {
ftruncateUnix(fd, (long)length);
} else {
context.LanguageContext.FileManager.GetStreams(fd).Truncate((long)length);
}
}


[SupportedOSPlatform("linux"), SupportedOSPlatform("osx")]
internal static void ftruncateUnix(int fd, long length) {
int result;
Mono.Unix.Native.Errno errno;
do {
result = Mono.Unix.Native.Syscall.ftruncate(fd, length);
} while (Mono.Unix.UnixMarshal.ShouldRetrySyscall(result, out errno));

if (errno != 0)
throw GetOsError(Mono.Unix.Native.NativeConvert.FromErrno(errno));
}


#if FEATURE_FILESYSTEM
public static object times() {
Expand Down
Loading
Loading