From 476ebae590c75784021df9800bd05b7a467912c7 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Tue, 3 Dec 2024 20:15:33 -0800 Subject: [PATCH 01/16] Fix indexing for arrays with non-zero base (#1824) * Fix indexing N-based system arrays * Support negative base arrays * Fix for Mono --- Src/IronPython/Runtime/Operations/ArrayOps.cs | 90 ++++++++++-- Tests/test_array.py | 138 ++++++++++++++++-- 2 files changed, 202 insertions(+), 26 deletions(-) diff --git a/Src/IronPython/Runtime/Operations/ArrayOps.cs b/Src/IronPython/Runtime/Operations/ArrayOps.cs index 87c4bcb62..60d2993b2 100644 --- a/Src/IronPython/Runtime/Operations/ArrayOps.cs +++ b/Src/IronPython/Runtime/Operations/ArrayOps.cs @@ -171,14 +171,14 @@ public static Array Multiply(Array data, int count) { if (data == null) throw PythonOps.TypeError("expected Array, got None"); if (data.Rank != 1) throw PythonOps.TypeError("bad dimensions for array, got {0} expected {1}", 1, data.Rank); - return data.GetValue(PythonOps.FixIndex(index, data.Length) + data.GetLowerBound(0)); + return data.GetValue(FixIndex(data, index, 0)); } [SpecialName] public static object GetItem(Array data, Slice slice) { if (data == null) throw PythonOps.TypeError("expected Array, got None"); - return GetSlice(data, data.Length, slice); + return GetSlice(data, slice); } [SpecialName] @@ -201,7 +201,6 @@ public static object GetItem(Array data, Slice slice) { int[] iindices = TupleToIndices(data, indices); if (data.Rank != indices.Length) throw PythonOps.TypeError("bad dimensions for array, got {0} expected {1}", indices.Length, data.Rank); - for (int i = 0; i < iindices.Length; i++) iindices[i] += data.GetLowerBound(i); return data.GetValue(iindices); } @@ -210,7 +209,7 @@ public static void SetItem(Array data, int index, object value) { if (data == null) throw PythonOps.TypeError("expected Array, got None"); if (data.Rank != 1) throw PythonOps.TypeError("bad dimensions for array, got {0} expected {1}", 1, data.Rank); - data.SetValue(Converter.Convert(value, data.GetType().GetElementType()), PythonOps.FixIndex(index, data.Length) + data.GetLowerBound(0)); + data.SetValue(Converter.Convert(value, data.GetType().GetElementType()), FixIndex(data, index, 0)); } [SpecialName] @@ -231,8 +230,6 @@ public static void SetItem(Array a, params object[] indexAndValue) { if (a.Rank != args.Length) throw PythonOps.TypeError("bad dimensions for array, got {0} expected {1}", args.Length, a.Rank); - for (int i = 0; i < indices.Length; i++) indices[i] += a.GetLowerBound(i); - Type elm = t.GetElementType()!; a.SetValue(Converter.Convert(indexAndValue[indexAndValue.Length - 1], elm), indices); } @@ -243,9 +240,15 @@ public static void SetItem(Array a, Slice index, object? value) { Type elm = a.GetType().GetElementType()!; + int lb = a.GetLowerBound(0); + if (lb != 0) { + FixSlice(index, a, out int start, out int stop, out int step); + index = new Slice(start - lb, stop - lb, step); + } + index.DoSliceAssign( delegate (int idx, object? val) { - a.SetValue(Converter.Convert(val, elm), idx + a.GetLowerBound(0)); + a.SetValue(Converter.Convert(val, elm), idx + lb); }, a.Length, value); @@ -375,10 +378,10 @@ public static string __repr__(CodeContext/*!*/ context, [NotNone] Array/*!*/ sel return GetSlice(data, start, stop, step); } - internal static Array GetSlice(Array data, int size, Slice slice) { + private static Array GetSlice(Array data, Slice slice) { if (data.Rank != 1) throw PythonOps.NotImplementedError("slice on multi-dimensional array"); - slice.Indices(size, out int start, out int stop, out int step); + FixSlice(slice, data, out int start, out int stop, out int step); if ((step > 0 && start >= stop) || (step < 0 && start <= stop)) { if (data.GetType().GetElementType() == typeof(object)) @@ -387,17 +390,17 @@ internal static Array GetSlice(Array data, int size, Slice slice) { return Array.CreateInstance(data.GetType().GetElementType()!, 0); } - if (step == 1) { + if (step == 1 && (!ClrModule.IsMono || data.GetLowerBound(0) == 0)) { int n = stop - start; Array ret = Array.CreateInstance(data.GetType().GetElementType()!, n); - Array.Copy(data, start + data.GetLowerBound(0), ret, 0, n); + Array.Copy(data, start, ret, 0, n); // doesn't work OK on Mono with non-0-based arrays return ret; } else { int n = PythonOps.GetSliceCount(start, stop, step); Array ret = Array.CreateInstance(data.GetType().GetElementType()!, n); int ri = 0; for (int i = 0, index = start; i < n; i++, index += step) { - ret.SetValue(data.GetValue(index + data.GetLowerBound(0)), ri++); + ret.SetValue(data.GetValue(index), ri++); } return ret; } @@ -427,11 +430,72 @@ private static int[] TupleToIndices(Array a, IList tuple) { int[] indices = new int[tuple.Count]; for (int i = 0; i < indices.Length; i++) { int iindex = Converter.ConvertToInt32(tuple[i]); - indices[i] = i < a.Rank ? PythonOps.FixIndex(iindex, a.GetLength(i)) : int.MinValue; + indices[i] = i < a.Rank ? FixIndex(a, iindex, i) : int.MinValue; } return indices; } + private static int FixIndex(Array a, int v, int axis) { + int idx = v; + int lb = a.GetLowerBound(axis); + int ub = a.GetUpperBound(axis); + if (idx < 0 && lb >= 0) { + idx += ub + 1; + } + if (idx < lb || idx > ub) { + throw PythonOps.IndexError("index out of range: {0}", v); + } + return idx; + } + + private static void FixSlice(Slice slice, Array a, out int ostart, out int ostop, out int ostep) { + Debug.Assert(a.Rank == 1); + + if (slice.step == null) { + ostep = 1; + } else { + ostep = Converter.ConvertToIndex(slice.step); + if (ostep == 0) { + throw PythonOps.ValueError("step cannot be zero"); + } + } + + int lb = a.GetLowerBound(0); + int ub = a.GetUpperBound(0); + + if (slice.start == null) { + ostart = ostep > 0 ? lb : ub; + } else { + ostart = Converter.ConvertToIndex(slice.start); + if (ostart < lb) { + if (ostart < 0 && lb >= 0) { + ostart += ub + 1; + } + if (ostart < lb) { + ostart = ostep > 0 ? lb : lb - 1; + } + } else if (ostart > ub) { + ostart = ostep > 0 ? ub + 1 : ub; + } + } + + if (slice.stop == null) { + ostop = ostep > 0 ? ub + 1 : lb - 1; + } else { + ostop = Converter.ConvertToIndex(slice.stop); + if (ostop < lb) { + if (ostop < 0 && lb >= 0) { + ostop += ub + 1; + } + if (ostop < 0) { + ostop = ostep > 0 ? lb : lb - 1; + } + } else if (ostop > ub) { + ostop = ostep > 0 ? ub + 1 : ub; + } + } + } + #endregion } } diff --git a/Tests/test_array.py b/Tests/test_array.py index 6287ff46a..97d0bc902 100644 --- a/Tests/test_array.py +++ b/Tests/test_array.py @@ -6,6 +6,26 @@ ## Test array support by IronPython (System.Array) ## +""" +Indexing of CLI arrays in IronPython: + + +| Base | Index >= 0 | Index < 0 | +|------|--------------------------------------|-------------------| +| > 0 | absolue | relative from end | +| 0 | absolute == relative from beginning | relative from end | +| < 0 | absolute | absolute | + +Comparison to indexing in C# and CPython: + +* Index >= 0, any base is C# compliant. +* Base 0, any index is CPython compliant. +* Base 0, index < 0 is not supported by C# but can be achieved by `System.Index` with 1-dim arrays only; then IronPython indexing is C# compliant. +* Base > 0, index < 0 is not supported by C#; IronPython follows CPython convention as more practical. +* Base < 0, index < 0 is C# compliant. +* Base != 0 is not supported by CPython for any builtin structures. +""" + from iptest import IronPythonTestCase, is_cli, run_test, skipUnlessIronPython if is_cli: @@ -146,6 +166,13 @@ def test_slice(self): def f(): array1[::2] = [x * 2 for x in range(11)] self.assertRaises(ValueError, f) + # slices on non-1-dim arrays are not supported + array2 = System.Array.CreateInstance(int, 20, 20) + self.assertRaises(NotImplementedError, lambda: array2[:]) # TODO: TypeError? + self.assertRaises(TypeError, lambda: array2[:, :]) # TODO: NotImplementedError? This would work in Numpy and Sympy + self.assertRaises(TypeError, lambda: array2[:, :, :]) # OK + + def test_creation(self): t = System.Array ti = type(System.Array.CreateInstance(int, 1)) @@ -173,35 +200,55 @@ def test_constructor(self): def test_nonzero_lowerbound(self): a = System.Array.CreateInstance(int, (5,), (5,)) - for i in range(5): a[i] = i + for i in range(5, 5 + a.Length): a[i] = i - self.assertEqual(a[:2], System.Array[int]((0,1))) - self.assertEqual(a[2:], System.Array[int]((2,3,4))) - self.assertEqual(a[2:4], System.Array[int]((2,3))) - self.assertEqual(a[-1], 4) + self.assertEqual(a[:7], System.Array[int]((5,6))) + self.assertEqual(a[7:], System.Array[int]((7,8,9))) + self.assertEqual(a[7:9], System.Array[int]((7,8))) + self.assertEqual(a[-1:-3:-1], System.Array[int]((9,8))) + self.assertEqual(a[-1], 9) - self.assertEqual(repr(a), 'Array[int]((0, 1, 2, 3, 4), base: 5)') + self.assertEqual(repr(a), 'Array[int]((5, 6, 7, 8, 9), base: 5)') a = System.Array.CreateInstance(int, (5,), (15,)) b = System.Array.CreateInstance(int, (5,), (20,)) self.assertEqual(a.Length, b.Length) for i in range(a.Length): - self.assertEqual(a[i], b[i]) + self.assertEqual(a[i + 15], b[i + 20]) + + a0 = System.Array.CreateInstance(int, 5) # regular, 0-based + for i in range(5): a0[i] = i - ## 5-dimension + a[17:19] = a0[2:4] + self.assertEqual(a[17:19], System.Array[int]((2, 3))) + + self.assertEqual(a0[3:1:-1], System.Array[int]((3, 2))) + self.assertEqual(a[18:16:-1], System.Array[int]((3, 2))) + + self.assertEqual(a0[-3:-1], System.Array[int]((2, 3))) + self.assertEqual(a[-3:-1], System.Array[int]((2, 3))) + + a[18:16:-1] = a0[2:4] + self.assertEqual(a[17:19], System.Array[int]((3, 2))) + + ## 5-dimension, 2-length/dim, progressive lowerbound a = System.Array.CreateInstance(int, (2,2,2,2,2), (1,2,3,4,5)) - self.assertEqual(a[0,0,0,0,0], 0) + self.assertEqual(a[1,2,3,4,5], 0) + + ## 5-dimension, 2-length/dim, progressive lowerbound + a = System.Array.CreateInstance(int, (2,2,2,2,2), (1,2,3,4,5)) + self.assertEqual(a[1,2,3,4,5], 0) for i in range(5): - index = [0,0,0,0,0] - index[i] = 1 + index = [1,2,3,4,5] + index[i] += 1 a[index[0], index[1], index[2], index[3], index[4]] = i self.assertEqual(a[index[0], index[1], index[2], index[3], index[4]], i) for i in range(5): - index = [0,0,0,0,0] - index[i] = 0 + index = [2,3,4,5,6] + index[i] -= 1 a[index[0], index[1], index[2], index[3], index[4]] = i self.assertEqual(a[index[0], index[1], index[2], index[3], index[4]], i) @@ -218,6 +265,71 @@ def sliceArrayAssign(arr, index, val): self.assertRaises(NotImplementedError, sliceArrayAssign, a, -200, 1) self.assertRaises(NotImplementedError, sliceArrayAssign, a, 1, 1) + def test_base1(self): + # For positive base arrays, indices are indexing elements directly (in absolute terms) + # rather than relative form the base. + # Negative indices are indexing relative form the end. + + # 1-based 2x2 matrix + arr = System.Array.CreateInstance(str, (2,2), (1,1)) + + self.assertEqual(arr.GetLowerBound(0), 1) + self.assertEqual(arr.GetLowerBound(1), 1) + self.assertEqual(arr.GetUpperBound(0), 2) + self.assertEqual(arr.GetUpperBound(1), 2) + + arr.SetValue("a_1,1", System.Array[System.Int32]((1,1))) + arr.SetValue("a_1,2", System.Array[System.Int32]((1,2))) + arr.SetValue("a_2,1", System.Array[System.Int32]((2,1))) + arr.SetValue("a_2,2", System.Array[System.Int32]((2,2))) + + self.assertEqual(arr[1, 1], "a_1,1") + self.assertEqual(arr[1, 2], "a_1,2") + self.assertEqual(arr[2, 1], "a_2,1") + self.assertEqual(arr[2, 2], "a_2,2") + + arr[1, 1] = "b_1,1" + arr[1, 2] = "b_1,2" + arr[2, 1] = "b_2,1" + arr[2, 2] = "b_2,2" + + self.assertEqual(arr.GetValue(System.Array[System.Int32]((1,1))), "b_1,1") + self.assertEqual(arr.GetValue(System.Array[System.Int32]((1,2))), "b_1,2") + self.assertEqual(arr.GetValue(System.Array[System.Int32]((2,1))), "b_2,1") + self.assertEqual(arr.GetValue(System.Array[System.Int32]((2,2))), "b_2,2") + + def test_base_negative(self): + # For negative base arrays, negative indices are indexing elements directly (like non negative indices) + # rather than indexing relative from the end. + + # 2-dim array [-1, 0, 1] x [-1, 0, 1] + arr = System.Array.CreateInstance(str, (3,3), (-1,-1)) + for i in range(-1, 2): + for j in range(-1, 2): + arr[i, j] = "a_%d,%d" % (i, j) + + for i in range(-1, 2): + for j in range(-1, 2): + self.assertEqual(arr[i, j], "a_%d,%d" % (i, j)) + + # test that VauleError is raised when the index is out of range + self.assertRaises(IndexError, lambda: arr[-2, 0]) + self.assertRaises(IndexError, lambda: arr[2, 0]) + self.assertRaises(IndexError, lambda: arr[0, -2]) + self.assertRaises(IndexError, lambda: arr[0, 2]) + + # test slice indexing + # 1-dim array [-1, 0, 1] + arr1 = System.Array.CreateInstance(int, (3,), (-1,)) + for i in range(-1, 2): + arr1[i] = i + self.assertEqual(arr1[-1:1], System.Array[int]((-1, 0))) + self.assertEqual(arr1[-2:1], System.Array[int]((-1, 0))) + self.assertEqual(arr1[0:], System.Array[int]((0, 1))) + self.assertEqual(arr1[:1], System.Array[int]((-1, 0))) + self.assertEqual(arr1[:], System.Array[int]((-1, 0, 1))) + self.assertEqual(arr1[:-2], System.Array[int](0)) + def test_array_type(self): def type_helper(array_type, instance): From 26c2dcaefcf9a178731141b0da677c2aad4e5641 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Wed, 4 Dec 2024 09:56:16 -0800 Subject: [PATCH 02/16] Correct error raised when slicing CLI arrays (#1827) --- Src/IronPython/Runtime/Operations/ArrayOps.cs | 6 +++++- Tests/test_array.py | 12 +++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Src/IronPython/Runtime/Operations/ArrayOps.cs b/Src/IronPython/Runtime/Operations/ArrayOps.cs index 60d2993b2..c58b2c435 100644 --- a/Src/IronPython/Runtime/Operations/ArrayOps.cs +++ b/Src/IronPython/Runtime/Operations/ArrayOps.cs @@ -429,7 +429,11 @@ private static Array GetSlice(Array data, Slice slice) { private static int[] TupleToIndices(Array a, IList tuple) { int[] indices = new int[tuple.Count]; for (int i = 0; i < indices.Length; i++) { - int iindex = Converter.ConvertToInt32(tuple[i]); + object? oindex = tuple[i]; + if (a.Rank != 1 && oindex is Slice) { + throw PythonOps.NotImplementedError("slice on multi-dimensional array"); + } + int iindex = Converter.ConvertToInt32(oindex); indices[i] = i < a.Rank ? FixIndex(a, iindex, i) : int.MinValue; } return indices; diff --git a/Tests/test_array.py b/Tests/test_array.py index 97d0bc902..4c71aaf55 100644 --- a/Tests/test_array.py +++ b/Tests/test_array.py @@ -166,12 +166,14 @@ def test_slice(self): def f(): array1[::2] = [x * 2 for x in range(11)] self.assertRaises(ValueError, f) - # slices on non-1-dim arrays are not supported + # slices on non-1D arrays are not supported yet array2 = System.Array.CreateInstance(int, 20, 20) - self.assertRaises(NotImplementedError, lambda: array2[:]) # TODO: TypeError? - self.assertRaises(TypeError, lambda: array2[:, :]) # TODO: NotImplementedError? This would work in Numpy and Sympy - self.assertRaises(TypeError, lambda: array2[:, :, :]) # OK - + # TODO: memoryview can slice ND views with a single slice + self.assertRaises(NotImplementedError, lambda: array2[:]) + # TODO: Numpy and Sympy can slice ND arrays with exactly N slices + self.assertRaises(NotImplementedError, lambda: array2[:, :]) + # TODO: Error matches memoryview; if slicing of ND arrays were supported, TypeError here would be expected + self.assertRaises(NotImplementedError, lambda: array2[:, :, :]) def test_creation(self): t = System.Array From 8504f29e04804d83d7804257168481417bdf0d0d Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Wed, 4 Dec 2024 14:27:22 -0800 Subject: [PATCH 03/16] Define UnsupportedOSPlatformAttribute --- Src/IronPython/SystemRuntimeVersioning.cs | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Src/IronPython/SystemRuntimeVersioning.cs b/Src/IronPython/SystemRuntimeVersioning.cs index d0c1ebe8d..e8c1a7ae5 100644 --- a/Src/IronPython/SystemRuntimeVersioning.cs +++ b/Src/IronPython/SystemRuntimeVersioning.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the Apache 2.0 license. // See the LICENSE file in the project root for more information. +#nullable enable + namespace System.Runtime.Versioning { #if !FEATURE_OSPLATFORMATTRIBUTE internal abstract class OSPlatformAttribute : Attribute { @@ -26,5 +28,27 @@ internal sealed class SupportedOSPlatformAttribute : OSPlatformAttribute { public SupportedOSPlatformAttribute(string platformName) : base(platformName) { } } + + [AttributeUsage(AttributeTargets.Assembly | + AttributeTargets.Class | + AttributeTargets.Constructor | + AttributeTargets.Enum | + AttributeTargets.Event | + AttributeTargets.Field | + AttributeTargets.Method | + AttributeTargets.Module | + AttributeTargets.Property | + AttributeTargets.Struct, + AllowMultiple = true, Inherited = false)] + internal sealed class UnsupportedOSPlatformAttribute : OSPlatformAttribute { + public UnsupportedOSPlatformAttribute(string platformName) : base(platformName) { + } + + public UnsupportedOSPlatformAttribute (string platformName, string? message) : base(platformName) { + Message = message; + } + + public string? Message { get; } + } #endif } From 84de081540be413a1e8736547f6dc1d0ffc5e63b Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Fri, 29 Nov 2024 20:07:43 -0800 Subject: [PATCH 04/16] Implement System.Array constructor with base arg --- Src/IronPython/Runtime/Operations/ArrayOps.cs | 16 +++++++----- Tests/test_array.py | 26 ++++++++++++++++--- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/Src/IronPython/Runtime/Operations/ArrayOps.cs b/Src/IronPython/Runtime/Operations/ArrayOps.cs index c58b2c435..0247f68e5 100644 --- a/Src/IronPython/Runtime/Operations/ArrayOps.cs +++ b/Src/IronPython/Runtime/Operations/ArrayOps.cs @@ -67,19 +67,23 @@ public static object __new__(CodeContext context, PythonType pythonType, ICollec } [StaticExtensionMethod] - public static object __new__(CodeContext context, PythonType pythonType, object items) { + public static object __new__(CodeContext context, PythonType pythonType, object items) + => __new__(context, pythonType, items, @base: 0); + + [StaticExtensionMethod] + public static object __new__(CodeContext context, PythonType pythonType, object items, /*[KeywordOnly]*/ int @base) { Type type = pythonType.UnderlyingSystemType.GetElementType()!; - object? lenFunc; - if (!PythonOps.TryGetBoundAttr(items, "__len__", out lenFunc)) + if (!PythonOps.TryGetBoundAttr(items, "__len__", out object? lenFunc)) throw PythonOps.TypeErrorForBadInstance("expected object with __len__ function, got {0}", items); int len = context.LanguageContext.ConvertToInt32(PythonOps.CallWithContext(context, lenFunc)); - Array res = Array.CreateInstance(type, len); + Array res = @base == 0 ? + Array.CreateInstance(type, len) : Array.CreateInstance(type, [len], [@base]); IEnumerator ie = PythonOps.GetEnumerator(items); - int i = 0; + int i = @base; while (ie.MoveNext()) { res.SetValue(Converter.Convert(ie.Current, type), i++); } @@ -277,7 +281,7 @@ public static string __repr__(CodeContext/*!*/ context, [NotNone] Array/*!*/ sel } ret.Append(')'); if (self.GetLowerBound(0) != 0) { - ret.Append(", base: "); + ret.Append(", base="); ret.Append(self.GetLowerBound(0)); } ret.Append(')'); diff --git a/Tests/test_array.py b/Tests/test_array.py index 4c71aaf55..d771a17dc 100644 --- a/Tests/test_array.py +++ b/Tests/test_array.py @@ -200,6 +200,26 @@ def test_constructor(self): for y in range(array3.GetLength(1)): self.assertEqual(array3[x, y], 0) + def test_constructor_nonzero_lowerbound(self): + # 1-based + arr = System.Array[int]((1, 2), base=1) + self.assertEqual(arr.Rank, 1) + self.assertEqual(arr.Length, 2) + self.assertEqual(arr.GetLowerBound(0), 1) + self.assertEqual(arr.GetUpperBound(0), 2) + self.assertEqual(arr[1], 1) + self.assertEqual(arr[2], 2) + for i in range(1, 3): + self.assertEqual(arr[i], i) + + def test_repr(self): + from System import Array + arr = Array[int]((5, 1), base=1) + s = repr(arr) + self.assertEqual(s, "Array[int]((5, 1), base=1)") + array4eval = eval(s, globals(), locals()) + self.assertEqual(arr, array4eval) + def test_nonzero_lowerbound(self): a = System.Array.CreateInstance(int, (5,), (5,)) for i in range(5, 5 + a.Length): a[i] = i @@ -210,7 +230,7 @@ def test_nonzero_lowerbound(self): self.assertEqual(a[-1:-3:-1], System.Array[int]((9,8))) self.assertEqual(a[-1], 9) - self.assertEqual(repr(a), 'Array[int]((5, 6, 7, 8, 9), base: 5)') + self.assertEqual(repr(a), 'Array[int]((5, 6, 7, 8, 9), base=5)') a = System.Array.CreateInstance(int, (5,), (15,)) b = System.Array.CreateInstance(int, (5,), (20,)) @@ -322,9 +342,7 @@ def test_base_negative(self): # test slice indexing # 1-dim array [-1, 0, 1] - arr1 = System.Array.CreateInstance(int, (3,), (-1,)) - for i in range(-1, 2): - arr1[i] = i + arr1 = System.Array[int]((-1, 0, 1), base=-1) self.assertEqual(arr1[-1:1], System.Array[int]((-1, 0))) self.assertEqual(arr1[-2:1], System.Array[int]((-1, 0))) self.assertEqual(arr1[0:], System.Array[int]((0, 1))) From 699824bd3ec52e052c1831348fd7971f584658eb Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Wed, 4 Dec 2024 20:05:43 -0800 Subject: [PATCH 05/16] Fix downward slices for positive base arrays --- Src/IronPython/Runtime/Operations/ArrayOps.cs | 2 +- Tests/test_array.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Src/IronPython/Runtime/Operations/ArrayOps.cs b/Src/IronPython/Runtime/Operations/ArrayOps.cs index 0247f68e5..e68e42d76 100644 --- a/Src/IronPython/Runtime/Operations/ArrayOps.cs +++ b/Src/IronPython/Runtime/Operations/ArrayOps.cs @@ -495,7 +495,7 @@ private static void FixSlice(Slice slice, Array a, out int ostart, out int ostop if (ostop < 0 && lb >= 0) { ostop += ub + 1; } - if (ostop < 0) { + if (ostop < lb) { ostop = ostep > 0 ? lb : lb - 1; } } else if (ostop > ub) { diff --git a/Tests/test_array.py b/Tests/test_array.py index d771a17dc..ccf9148e5 100644 --- a/Tests/test_array.py +++ b/Tests/test_array.py @@ -497,4 +497,20 @@ def test_equality_rank(self): self.assertTrue(c != d) self.assertFalse(d != d1) + def test_slice_down(self): + #arr = System.Array[int]((2, 3, 4), base=2) + arr = System.Array.CreateInstance(int, (3,), (2,)) + for i in range(2, 2 + 3): + arr[i] = i + + self.assertEqual(arr[-1:-4:-1], System.Array[int]((4, 3, 2))) + self.assertEqual(arr[-1:-5:-1], System.Array[int]((4, 3, 2))) + self.assertEqual(arr[-1:-6:-1], System.Array[int]((4, 3, 2))) + + self.assertEqual(arr[2:1:-1], System.Array[int]((2,))) + self.assertEqual(arr[2:-5:-1], System.Array[int]((2,))) + self.assertEqual(arr[2:-6:-1], System.Array[int]((2,))) + self.assertEqual(arr[1:-5:-1], System.Array[int](())) + self.assertEqual(arr[0:-5:-1], System.Array[int](())) + run_test(__name__) From 2e458f0be7e71cb29618f9d78642b012430584a7 Mon Sep 17 00:00:00 2001 From: slozier Date: Mon, 9 Dec 2024 09:01:37 -0500 Subject: [PATCH 06/16] Fix CVE-2015-20107 (#1833) * Fix CVE-2015-20107 * Fix tests --- Src/StdLib/Lib/mailcap.py | 29 +++++++++++++++++++++++++++-- Src/StdLib/Lib/test/test_mailcap.py | 10 +++++++--- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/Src/StdLib/Lib/mailcap.py b/Src/StdLib/Lib/mailcap.py index 97e303522..4cbda7ac0 100644 --- a/Src/StdLib/Lib/mailcap.py +++ b/Src/StdLib/Lib/mailcap.py @@ -1,9 +1,18 @@ """Mailcap file handling. See RFC 1524.""" import os +import warnings +import re __all__ = ["getcaps","findmatch"] +# https://github.com/IronLanguages/ironpython3/issues/1274 +# _find_unsafe = re.compile(r'[^\xa1-\U0010FFFF\w@+=:,./-]').search +_find_unsafe = re.compile(r'[^\w@+=:,./-]').search + +class UnsafeMailcapInput(Warning): + """Warning raised when refusing unsafe input""" + # Part 1: top-level interface. def getcaps(): @@ -144,15 +153,22 @@ def findmatch(caps, MIMEtype, key='view', filename="/dev/null", plist=[]): entry to use. """ + if _find_unsafe(filename): + msg = "Refusing to use mailcap with filename %r. Use a safe temporary filename." % (filename,) + warnings.warn(msg, UnsafeMailcapInput) + return None, None entries = lookup(caps, MIMEtype, key) # XXX This code should somehow check for the needsterminal flag. for e in entries: if 'test' in e: test = subst(e['test'], filename, plist) + if test is None: + continue if test and os.system(test) != 0: continue command = subst(e[key], MIMEtype, filename, plist) - return command, e + if command is not None: + return command, e return None, None def lookup(caps, MIMEtype, key=None): @@ -184,6 +200,10 @@ def subst(field, MIMEtype, filename, plist=[]): elif c == 's': res = res + filename elif c == 't': + if _find_unsafe(MIMEtype): + msg = "Refusing to substitute MIME type %r into a shell command." % (MIMEtype,) + warnings.warn(msg, UnsafeMailcapInput) + return None res = res + MIMEtype elif c == '{': start = i @@ -191,7 +211,12 @@ def subst(field, MIMEtype, filename, plist=[]): i = i+1 name = field[start:i] i = i+1 - res = res + findparam(name, plist) + param = findparam(name, plist) + if _find_unsafe(param): + msg = "Refusing to substitute parameter %r (%s) into a shell command" % (param, name) + warnings.warn(msg, UnsafeMailcapInput) + return None + res = res + param # XXX To do: # %n == number of parts if type is multipart/* # %F == list of alternating type and filename for parts diff --git a/Src/StdLib/Lib/test/test_mailcap.py b/Src/StdLib/Lib/test/test_mailcap.py index a4cd09c70..3e774a385 100644 --- a/Src/StdLib/Lib/test/test_mailcap.py +++ b/Src/StdLib/Lib/test/test_mailcap.py @@ -101,8 +101,9 @@ def test_subst(self): (["", "audio/*", "foo.txt"], ""), (["echo foo", "audio/*", "foo.txt"], "echo foo"), (["echo %s", "audio/*", "foo.txt"], "echo foo.txt"), - (["echo %t", "audio/*", "foo.txt"], "echo audio/*"), - (["echo \%t", "audio/*", "foo.txt"], "echo %t"), + (["echo %t", "audio/*", "foo.txt"], None), + (["echo %t", "audio/wav", "foo.txt"], "echo audio/wav"), + (["echo \\%t", "audio/*", "foo.txt"], "echo %t"), (["echo foo", "audio/*", "foo.txt", plist], "echo foo"), (["echo %{total}", "audio/*", "foo.txt", plist], "echo 3") ] @@ -183,7 +184,10 @@ def test_findmatch(self): ('"An audio fragment"', audio_basic_entry)), ([c, "audio/*"], {"filename": fname}, - ("/usr/local/bin/showaudio audio/*", audio_entry)), + (None, None)), + ([c, "audio/wav"], + {"filename": fname}, + ("/usr/local/bin/showaudio audio/wav", audio_entry)), ([c, "message/external-body"], {"plist": plist}, ("showexternal /dev/null default john python.org /tmp foo bar", message_entry)) From bde56ce3dc5072390d34b9a566b498684b9564e9 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Mon, 9 Dec 2024 20:14:05 -0800 Subject: [PATCH 07/16] Give more time in test__socket for server to start up (#1836) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Give more time in test__socket for server to start up * Clean up resources and speed up test --------- Co-authored-by: Stéphane Lozier --- Tests/modules/network_related/test__socket.py | 80 +++++++++++-------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/Tests/modules/network_related/test__socket.py b/Tests/modules/network_related/test__socket.py index 626be3d50..ce1ba4548 100644 --- a/Tests/modules/network_related/test__socket.py +++ b/Tests/modules/network_related/test__socket.py @@ -419,8 +419,6 @@ def test_getfqdn(self): pass def test_cp5814(self): - global EXIT_CODE - global HAS_EXITED EXIT_CODE = -1 HAS_EXITED = False @@ -428,7 +426,6 @@ def test_cp5814(self): #Server code server = """ -from time import sleep import _socket import os @@ -454,12 +451,12 @@ def test_cp5814(self): #Verifications if not addr[0] in [HOST, '127.0.0.1']: raise Exception('The address, %s, was unexpected' % str(addr)) - if data!=b'stuff': - raise Exception('%s!=stuff' % str(data)) - sleep(10) + if data != b'stuff': + raise Exception('%s != stuff' % str(data)) finally: conn.close() + s.close() try: os.remove(r"{PORTFILE}") except: @@ -467,8 +464,8 @@ def test_cp5814(self): """.format(PORTFILE=portFile) #Spawn off a thread to startup the server def server_thread(): - global EXIT_CODE - global HAS_EXITED + nonlocal EXIT_CODE + nonlocal HAS_EXITED serverFile = os.path.join(self.temporary_dir, "cp5814server_%d.py" % os.getpid()) self.write_to_file(serverFile, server) EXIT_CODE = os.system('"%s" %s' % @@ -484,7 +481,7 @@ def server_thread(): portex = None startTime = time.perf_counter() for _ in range(20): - time.sleep(1) + time.sleep(0.5) if EXIT_CODE > 0: self.fail("Server died with exit code %d" % EXIT_CODE) try: @@ -506,13 +503,14 @@ def server_thread(): s.close() #Ensure the server didn't die - for i in range(100): - if not HAS_EXITED: - print("*", end="") - time.sleep(1) - else: + for _ in range(100): + if HAS_EXITED: self.assertEqual(EXIT_CODE, 0) break + + print("*", end="") + time.sleep(0.5) + self.assertTrue(HAS_EXITED) #Verification @@ -526,29 +524,38 @@ def server_thread(): class SocketMakefileTest(IronPythonTestCase): def test_misc(self): - f = socket.socket().makefile() + s = socket.socket() + f = s.makefile() f.bufsize = 4096 self.assertEqual(4096, f.bufsize) + f.close() + s.close() def test_makefile_refcount(self): "Ensures that the _socket stays open while there's still a file associated" - global PORT + GPORT = None def echoer(): - global PORT + nonlocal GPORT s = socket.socket() s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) # prevents an "Address already in use" error when the socket is in a TIME_WAIT state s.settimeout(15) # prevents the server from staying open if the client never connects s.bind(('localhost', 0)) - PORT = s.getsockname()[1] + GPORT = s.getsockname()[1] s.listen(5) - (s2, ignore) = s.accept() + (s2, _) = s.accept() s2.send(s2.recv(10)) + s2.close() + s.close() _thread.start_new_thread(echoer, ()) - time.sleep(1) + for _ in range(20): + time.sleep(0.5) + if GPORT is not None: + break + s = socket.socket() - s.connect(('localhost', PORT)) + s.connect(('localhost', GPORT)) f1 = s.makefile('r') f2 = s.makefile('w') s.close() @@ -558,9 +565,10 @@ def echoer(): str = f1.readline() self.assertEqual(str, test_msg) + f2.close() + f1.close() + def test_cp7451(self): - global EXIT_CODE - global HAS_EXITED EXIT_CODE = -1 HAS_EXITED = False @@ -568,7 +576,6 @@ def test_cp7451(self): #Server code server = """ -from time import sleep import socket as _socket import os @@ -593,12 +600,12 @@ def test_cp7451(self): #Verifications if not addr[0] in [HOST, '127.0.0.1']: raise Exception('The address, %s, was unexpected' % str(addr)) - if data!=b'stuff2': - raise Exception('%s!=stuff2' % str(data)) - sleep(10) + if data != b'stuff2': + raise Exception('%s != stuff2' % str(data)) finally: conn.close() + s.close() try: os.remove(r"{PORTFILE}") except: @@ -606,8 +613,8 @@ def test_cp7451(self): """.format(PORTFILE=portFile) #Spawn off a thread to startup the server def server_thread(): - global EXIT_CODE - global HAS_EXITED + nonlocal EXIT_CODE + nonlocal HAS_EXITED serverFile = os.path.join(self.temporary_dir, "cp7451server_%d.py" % os.getpid()) self.write_to_file(serverFile, server) EXIT_CODE = os.system('"%s" %s' % @@ -623,7 +630,7 @@ def server_thread(): portex = None startTime = time.perf_counter() for _ in range(20): - time.sleep(1) + time.sleep(0.5) if EXIT_CODE > 0: self.fail("Server died with exit code %d" % EXIT_CODE) try: @@ -645,16 +652,19 @@ def server_thread(): s.close() #Ensure the server didn't die - for i in range(100): - if not HAS_EXITED: - print("*", end="") - time.sleep(1) - else: + for _ in range(100): + if HAS_EXITED: self.assertEqual(EXIT_CODE, 0) break + + print("*", end="") + time.sleep(0.5) + self.assertTrue(HAS_EXITED) #Verification self.assertEqual(f.read(6), "stuff2") + f.close() + run_test(__name__) From 517c5483b3a6bf8bf73967ea4f8e406df7b7fab6 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Wed, 11 Dec 2024 08:01:40 -0800 Subject: [PATCH 08/16] Autodetect processor architecture used for tests (#1837) * Autodetect processor architecture used for tests * Use dotnet runner autodetect mechanism --- make.ps1 | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/make.ps1 b/make.ps1 index 16e082c1b..32786dd37 100755 --- a/make.ps1 +++ b/make.ps1 @@ -5,7 +5,7 @@ Param( [String] $target = "build", [String] $configuration = "Release", [String[]] $frameworks=@('net462','netcoreapp3.1','net6.0','net8.0'), - [String] $platform = "x64", + [String] $platform = $null, # auto-detect [switch] $runIgnored, [int] $jobs = [System.Environment]::ProcessorCount ) @@ -62,8 +62,12 @@ function GenerateRunSettings([String] $framework, [String] $platform, [String] $ [System.Xml.XmlDocument]$doc = New-Object System.Xml.XmlDocument # +# +# x64 +# # # +# # # @@ -71,12 +75,14 @@ function GenerateRunSettings([String] $framework, [String] $platform, [String] $ $doc.AppendChild($dec) | Out-Null $runSettings = $doc.CreateElement("RunSettings") - + $runConfiguration = $doc.CreateElement("RunConfiguration") $runSettings.AppendChild($runConfiguration) | Out-Null - $targetPlatform = $doc.CreateElement("TargetPlatform") - $targetPlatform.InnerText = $platform - $runConfiguration.AppendChild($targetPlatform) | Out-Null + if ($platform) { + $targetPlatform = $doc.CreateElement("TargetPlatform") + $targetPlatform.InnerText = $platform + $runConfiguration.AppendChild($targetPlatform) | Out-Null + } $testRunParameters = $doc.CreateElement("TestRunParameters") $runSettings.AppendChild($testRunParameters) | Out-Null From f971ff676e2349971868780bb1c5d49a2a5af32e Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Wed, 11 Dec 2024 16:02:58 -0800 Subject: [PATCH 09/16] Use genuine file descriptors on Posix (#1832) * Use genuine file descriptors on Unix * Complete implementing genuine file descriptors * Add developer's deocumentation * Update after review * Add wbplus file test --- Documentation/file-descriptors.md | 144 ++++++++++++++++++++ Src/IronPython.Modules/nt.cs | 115 ++++++++++++---- Src/IronPython/Modules/_fileio.cs | 2 +- Src/IronPython/Runtime/PythonFileManager.cs | 44 ++++-- Tests/modules/io_related/test_fd.py | 51 +++---- Tests/test_file.py | 23 +++- 6 files changed, 315 insertions(+), 64 deletions(-) create mode 100755 Documentation/file-descriptors.md diff --git a/Documentation/file-descriptors.md b/Documentation/file-descriptors.md new file mode 100755 index 000000000..da884bb75 --- /dev/null +++ b/Documentation/file-descriptors.md @@ -0,0 +1,144 @@ +# File Descriptors in IronPython + +## Windows + +The conceptual picture of file descriptors (FDs) usage on Windows, for the most interesting case of `FileStream`: + +```mermaid +graph LR; + +FileIO --> StreamBox --> FileStream --> Handle(Handle) --> OSFile[OS File]; +FD(FD) <--> StreamBox; +``` + +Conceptually, the relationship between `FD` (a number) and `StreamBox` (a class) is bidirectional because `PythonFileManager` (global singleton) maintains the association between the two so it is cost-free to obtaining the one having the other. FD is not the same as the handle, which is created by the OS. FD is an emulated (fake) file descriptor, assigned by the `PythonFileManager`, for the purpose of supporting the Python API. The descriptors are allocated lazily, i.e. only if the user code makes an API call that accesses it. Once assigned, the descriptor does not change. The FD number is released once the FD is closed (or the associated `FileIO` is closed and had `closefd` set to true.) + +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. + +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. + +```mermaid +graph LR; + +FD1(FD1) <--> StreamBox --> FileStream --> Handle(Handle) --> OSFile[OS File]; +FD2(FD2) <--> StreamBox2[StreamBox] --> FileStream; +``` + +The descriptors can be closed independently, and the underlying `FileStream` is closed when the last `StreamBox` using it is closed. + +## Posix + +On Unix-like systems (Linux, maxOS), `FileStream` uses the actual file descriptor as the handle. In the past. IronPython was ignoring this and still issuing its own fake file descriptors as it is in the case of Windows. Now, however, the genuine FD is extracted from the handle and used as FD at the `PythonFileManager` level, ensuring that clients of Python API obtain the genuine FD. + +```mermaid +graph LR; + +FileIO --> StreamBox --> FileStream --> FDH(FD) --> OSFile[OS File]; +FD(FD) <--> StreamBox; +``` + +When descriptor FD is duplicated, the actual OS call is made to create the duplicate FD2. In order to use FD2 directly, a new `Stream` object has to be created around it. + +### Optimal Mechanism + +The optimal solution is to create another `FileStream` using the constructor that accepts an already opened file descriptor. + +```mermaid +graph LR; + +FD1(FD1) <--> StreamBox --> FileStream --> FDH1(FD1) --> OSFile[OS File]; +FD2(FD2) <--> StreamBox2[StreamBox] --> FileStream2[FileStream] --> FDH2(FD2) --> OSFile; +``` + +In this way, the file descriptor on the `PythonFileManager` level is the same as the file descriptor used by `FileStream`. + +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. + +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. + +### Mono Workaround + +To use system-opened file descriptors on Mono `UnixStream` can be used instead of `FileStream`. + +```mermaid +graph LR; + +FD1(FD1) <--> StreamBox --> FileStream --> FDH1(FD1) --> OSFile[OS File]; +FD2(FD2) <--> StreamBox2[StreamBox] --> UnixStream --> FDH2(FD2) --> OSFile; +``` + +Since FileIO works with various types of the underlying `Stream`, using `UnixStream` should be OK. + +Although `UnixStream` is available in .NET through package `Mono.Posix`, this solution still does not work around desynchronized read/write position, which `FileStream` using the original FD1 must somehow maintain independently. + +### .NET Workaround + +To ensure proper R/W behavior on .NET, operations on both file descriptions have to go though the same `FileStream` object. Since the duplicated file descriptor is basically just a number, pointing to the same file description as the original descriptor, on the OS level it doesn't matter which descriptor is used for operations. The only difference between those descriptors is flag `O_CLOEXEC`, which determines whether the descriptor stays open or not when child processed are executed. + +```mermaid +graph LR; + +FD1(FD1) <--> StreamBox --> FileStream --> FDH1(FD1) --> OSFile[OS File]; +FD2(FD2) <--> StreamBox2[StreamBox] --> FileStream; +FDH2(FD2) --> OSFile; +``` + +This actually works OK, until `dup2` is used. When the FD1 descriptor (or the associated `FileIO`) is closed on the Python API level, the underlying OS descriptor is not released but still being used by `FileStream`. A small side effect is that it will not be reused until FD2 is closed, but other than that, the behaviour is as expected. + +```mermaid +graph LR; + +FileStream --> FDH1(FD1) --> OSFile[OS File]; +FD2(FD2) <--> StreamBox2[StreamBox] --> FileStream; +FDH2(FD2) --> OSFile; +``` + +The problem arises when `dup2` is used with the target being FD1. This will forcibly close the descriptor used by `FileStream`, rendering the stream broken, despite having FD2 available. Perhaps closing `FileStream` using FD1 and opening a replacement around FD2 could be a solution, but this would have to be done atomically. If so, this would lead to a healthy structure. + +```mermaid +graph LR; + +FileStream --> FDH2(FD2); +FD2(FD2) <--> StreamBox2[StreamBox] --> FileStream; +FDH2(FD2) --> OSFile; +``` + + +## Practical Scenarios + +None of the above solutions is fully satisfactory for .NET. Ideally, .NET would behave consistently with Posix, because even the most elaborate workarounds (like juggling various `FileStream` objects around the descriptors) only work within IronPython, and break down when a descriptor is passed to a 3rd party library that uses C extension and creates its own `FILE*` struct around it. The `FileStream` object in .NET knows nothing about it and will not adjust its R/W position. + +In the meantime, let's look at some practical cases when `dup`/`dup2` are used and try to support just these. For what I have seen, `dup`/`dup2` are commonly used to redirect some of the standard descriptors. For example, to redirect standard output to a file: +1. Open a file for writing, it will get assigned descriptor FD1. +2. Copy descriptor 1 aside using `dup`. The copy will get assigned descriptor FD2. +3. Copy the open file descriptor FD1 onto descriptor 1 using `dup2`. This will forcibly close the existing descriptor 1, but not the output stream, which is sill accessible through descriptor FD2. +4. Code writing to "standard output", i.e. descriptor 1, will now write to the open file. +5. If needed, the application can still write to the original output stream by writing to descriptor FD2. +6. When done, close descriptor FD1. +7. Copy descriptor FD2 onto descriptor 1 using `dup2`. Since the is the last one pointing to the open file, the file will be closed as well. +8. Close descriptor FD2, the copy is not needed anymore. + +The same scenario is commonly done for standard input and sometimes standard error. + +The problem of .NET manifests itself when there are two descriptors open that refer to the same open file description and used concurrently. In the above scenario it is descriptor 1 and FD1. Assuming that the application is not using FD1 (typical use), the _Optimal Mechanism_ described above is sufficient. + +If the application does insist on using both descriptors 1 and FD1, the first .NET workaround is needed. This will lead to the following structure: + +```mermaid +graph LR; + +FD1(FD1) <--> StreamBox --> FileStream --> FDH1(FD1) --> OSFile[OS File]; +D1(1) <--> StreamBox2[StreamBox] --> FileStream; +DH1(1) --> OSFile; +FD2(FD2) <--> StreamBox3[StreamBox] --> FileStream2[FileStream] --> FDH2(FD2) --> stdout +``` + +The problem of closing FD1 and then overwriting it is not an issue, since only standard descriptors (0, 1, 2) are being overwritten with `dup2`. There is still a problem of overwriting data written by C extension code writing though descriptor 1. Perhaps replacing `FileStream` utilizing FD1 with `UnixStream` from Mono would make it more cooperative. + +In the end, the implementation of genuine file descriptors in IronPython starts with the simple solution (the simple workarounds described above) and will be adjusted as needed to support the 3rd party Python packages. + +## Special Case: Double Stream + +In Python, a file can be opened with mode "ab+". The file is opened for appending to the end (created if not exists), and the `+` means that it is also opened for updating. i.e. reading and writing. The file pointer is initially set at the end of the file (ready to write to append) but can be moved around to read already existing data. However, each write will append data to the end and reset the read/write pointer at the end again. In IronPython this is simulated by using two file streams, one for reading and one fore writing. Both are maintained in a single `StreamBox` but will have different file descriptors. This is subject to change. + +[1]: https://github.com/mono/mono/issues/12783 diff --git a/Src/IronPython.Modules/nt.cs b/Src/IronPython.Modules/nt.cs index f6f920a3c..8d91d5438 100644 --- a/Src/IronPython.Modules/nt.cs +++ b/Src/IronPython.Modules/nt.cs @@ -351,9 +351,25 @@ public static int dup(CodeContext/*!*/ context, int fd) { PythonFileManager fileManager = context.LanguageContext.FileManager; StreamBox streams = fileManager.GetStreams(fd); // OSError if fd not valid - fileManager.EnsureRefStreams(streams); - fileManager.AddRefStreams(streams); - return fileManager.Add(new(streams)); + 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)); + } else { + // Share the same set of streams between the original and the dupped descriptor + fileManager.EnsureRefStreams(streams); + fileManager.AddRefStreams(streams); + return fileManager.Add(fd2, new(streams)); + } + } else { + fileManager.EnsureRefStreams(streams); + fileManager.AddRefStreams(streams); + return fileManager.Add(new(streams)); + } } @@ -373,11 +389,45 @@ 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 + // TODO: race condition: `open` or `dup` on another thread may occupy fd2 (simulated descriptors only) + + 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; + } + fd2 = UnixDup(fd, fd2, out Stream? dupstream); // closes fd2 atomically if reopened in the meantime + fileManager.Remove(fd2); + if (dupstream is not null) { + return fileManager.Add(fd2, new(dupstream)); + } else { + // Share the same set of streams between the original and the dupped descriptor + fileManager.EnsureRefStreams(streams); + fileManager.AddRefStreams(streams); + return fileManager.Add(fd2, new(streams)); + } + } + } + - fileManager.EnsureRefStreams(streams); - fileManager.AddRefStreams(streams); - return fileManager.Add(fd2, new(streams)); + 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); + } 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 + } + return res; } #if FEATURE_PROCESS @@ -827,22 +877,28 @@ public static object open(CodeContext/*!*/ context, [NotNone] string path, int f FileMode fileMode = FileModeFromFlags(flags); FileAccess access = FileAccessFromFlags(flags); FileOptions options = FileOptionsFromFlags(flags); - Stream fs; + Stream s; + FileStream? fs; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && IsNulFile(path)) { - fs = Stream.Null; + fs = null; + s = Stream.Null; } else if (access == FileAccess.Read && (fileMode == FileMode.CreateNew || fileMode == FileMode.Create || fileMode == FileMode.Append)) { // .NET doesn't allow Create/CreateNew w/ access == Read, so create the file, then close it, then // open it again w/ just read access. fs = new FileStream(path, fileMode, FileAccess.Write, FileShare.None); fs.Close(); - fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, DefaultBufferSize, options); + s = fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, DefaultBufferSize, options); } else if (access == FileAccess.ReadWrite && fileMode == FileMode.Append) { - fs = new FileStream(path, FileMode.Append, FileAccess.Write, FileShare.ReadWrite, DefaultBufferSize, options); + s = fs = new FileStream(path, FileMode.Append, FileAccess.Write, FileShare.ReadWrite, DefaultBufferSize, options); } else { - fs = new FileStream(path, fileMode, access, FileShare.ReadWrite, DefaultBufferSize, options); + s = fs = new FileStream(path, fileMode, access, FileShare.ReadWrite, DefaultBufferSize, options); } - return context.LanguageContext.FileManager.Add(new(fs)); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { + return context.LanguageContext.FileManager.Add(new(s)); + } else { + return context.LanguageContext.FileManager.Add((int)fs!.SafeFileHandle.DangerousGetHandle(), new(s)); + } } catch (Exception e) { throw ToPythonException(e, path); } @@ -877,30 +933,29 @@ private static FileOptions FileOptionsFromFlags(int flag) { #if FEATURE_PIPES - private static Tuple CreatePipeStreams() { - if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) || RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - return CreatePipeStreamsUnix(); - } else { + public static PythonTuple pipe(CodeContext context) { + var manager = context.LanguageContext.FileManager; + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { var inPipe = new AnonymousPipeServerStream(PipeDirection.In); var outPipe = new AnonymousPipeClientStream(PipeDirection.Out, inPipe.ClientSafePipeHandle); - return Tuple.Create(inPipe, outPipe); + return PythonTuple.MakeTuple( + manager.Add(new(inPipe)), + manager.Add(new(outPipe)) + ); + } else { + var pipeStreams = CreatePipeStreamsUnix(); + return PythonTuple.MakeTuple( + manager.Add(pipeStreams.Item1, new(pipeStreams.Item2)), + manager.Add(pipeStreams.Item3, new(pipeStreams.Item4)) + ); } - static Tuple CreatePipeStreamsUnix() { + static Tuple CreatePipeStreamsUnix() { Mono.Unix.UnixPipes pipes = Mono.Unix.UnixPipes.CreatePipes(); - return Tuple.Create(pipes.Reading, pipes.Writing); + return Tuple.Create(pipes.Reading.Handle, pipes.Reading, pipes.Writing.Handle, pipes.Writing); } } - - public static PythonTuple pipe(CodeContext context) { - var pipeStreams = CreatePipeStreams(); - var manager = context.LanguageContext.FileManager; - - return PythonTuple.MakeTuple( - manager.Add(new(pipeStreams.Item1)), - manager.Add(new(pipeStreams.Item2)) - ); - } #endif #if FEATURE_PROCESS diff --git a/Src/IronPython/Modules/_fileio.cs b/Src/IronPython/Modules/_fileio.cs index 1069ea15b..93e2135bf 100644 --- a/Src/IronPython/Modules/_fileio.cs +++ b/Src/IronPython/Modules/_fileio.cs @@ -239,7 +239,7 @@ static Exception BadMode(string mode) { [Documentation("close() -> None. Close the file.\n\n" + "A closed file cannot be used for further I/O operations. close() may be" - + "called more than once without error. Changes the fileno to -1." + + "called more than once without error." )] public override void close(CodeContext/*!*/ context) { if (_closed) { diff --git a/Src/IronPython/Runtime/PythonFileManager.cs b/Src/IronPython/Runtime/PythonFileManager.cs index 5070b9ffa..64a81bb9b 100644 --- a/Src/IronPython/Runtime/PythonFileManager.cs +++ b/Src/IronPython/Runtime/PythonFileManager.cs @@ -10,6 +10,8 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Runtime.InteropServices; +using System.Runtime.Versioning; using Microsoft.Scripting.Runtime; using Microsoft.Scripting.Utils; @@ -86,13 +88,16 @@ public bool IsConsoleStream() { // Isolate Mono.Unix from the rest of the method so that we don't try to load the Mono.Unix assembly on Windows. bool isattyUnix() { - // TODO: console streams may be dupped to differend FD numbers, do not use hard-coded 0, 1, 2 - return StreamType switch { - ConsoleStreamType.Input => Mono.Unix.Native.Syscall.isatty(0), - ConsoleStreamType.Output => Mono.Unix.Native.Syscall.isatty(1), - ConsoleStreamType.ErrorOutput => Mono.Unix.Native.Syscall.isatty(2), - _ => false - }; + if (Id >= 0) { + return Mono.Unix.Native.Syscall.isatty(Id); + } else { + return StreamType switch { + ConsoleStreamType.Input => Mono.Unix.Native.Syscall.isatty(0), + ConsoleStreamType.Output => Mono.Unix.Native.Syscall.isatty(1), + ConsoleStreamType.ErrorOutput => Mono.Unix.Native.Syscall.isatty(2), + _ => false + }; + } } } @@ -212,6 +217,8 @@ internal class PythonFileManager { private int _current = _offset; // lowest potentially unused key in _objects at or above _offset private readonly ConcurrentDictionary _refs = new(); + // This version of Add is used with genuine file descriptors (Unix). + // Exception: support dup2 on all frameworks/platfroms. public int Add(int id, StreamBox streams) { ContractUtils.RequiresNotNull(streams, nameof(streams)); ContractUtils.Requires(streams.Id < 0, nameof(streams)); @@ -226,6 +233,9 @@ public int Add(int id, StreamBox streams) { } } + // This version of Add is used for emulated file descriptors. + // Must not be used on POSIX. + [UnsupportedOSPlatform("linux"), UnsupportedOSPlatform("macos")] public int Add(StreamBox streams) { ContractUtils.RequiresNotNull(streams, nameof(streams)); ContractUtils.Requires(streams.Id < 0, nameof(streams)); @@ -277,7 +287,13 @@ public int GetOrAssignId(StreamBox streams) { lock (_synchObject) { int res = streams.Id; if (res == -1) { - res = Add(streams); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { + res = Add(streams); + } else { + res = GetGenuineFileDescriptor(streams.ReadStream); + if (res < 0) throw new InvalidOperationException("stream not associated with a file descriptor"); + Add(res, streams); + } } return res; } @@ -310,5 +326,17 @@ public bool DerefStreamsAndCloseIfLast(StreamBox streams) { public bool ValidateFdRange(int fd) { return fd >= 0 && fd < LIMIT_OFILE; } + + [UnsupportedOSPlatform("windows")] + private static int GetGenuineFileDescriptor(Stream stream) { + return stream switch { + FileStream fs => checked((int)fs.SafeFileHandle.DangerousGetHandle()), +#if FEATURE_PIPES + System.IO.Pipes.PipeStream ps => checked((int)ps.SafePipeHandle.DangerousGetHandle()), +#endif + Mono.Unix.UnixStream us => us.Handle, + _ => -1 + }; + } } } diff --git a/Tests/modules/io_related/test_fd.py b/Tests/modules/io_related/test_fd.py index 608643d60..681cdcc46 100644 --- a/Tests/modules/io_related/test_fd.py +++ b/Tests/modules/io_related/test_fd.py @@ -9,7 +9,7 @@ import sys import unittest -from iptest import IronPythonTestCase, is_cli, run_test +from iptest import IronPythonTestCase, is_cli, is_posix, run_test from threading import Timer flags = os.O_CREAT | os.O_TRUNC | os.O_RDWR @@ -45,12 +45,11 @@ def test_dup2(self): # Assigning to self must be a no-op. self.assertEqual(os.dup2(fd, fd), fd if is_cli or sys.version_info >= (3,7) else None) - self.assertTrue(is_open(fd)) # The source must be valid. - self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor", os.dup2, -1, fd) + self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor" if is_cli or sys.version_info >= (3,5) else "[Errno 0] Error", os.dup2, -1, fd) self.assertTrue(is_open(fd)) - self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor", os.dup2, 99, fd) + self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor", os.dup2, fd + 10000, fd) self.assertTrue(is_open(fd)) # If the source is not open, then the destination is unaffected. @@ -67,16 +66,16 @@ def test_dup2(self): # Using dup2 can skip fds. self.assertEqual(os.dup2(fd, fd + 2), fd + 2 if is_cli or sys.version_info >= (3,7) else None) self.assertTrue(is_open(fd)) - self.assertTrue(not is_open(fd + 1)) + self.assertFalse(is_open(fd + 1)) self.assertTrue(is_open(fd + 2)) # Verify that dup2 closes the previous occupant of a fd. - self.assertEqual(os.open(os.devnull, os.O_RDWR, 0o600), fd + 1) - self.assertEqual(os.dup2(fd + 1, fd), fd if is_cli or sys.version_info >= (3,7) else None) + fdn = os.open(os.devnull, os.O_RDWR, 0o600) + self.assertEqual(os.dup2(fdn, fd), fd if is_cli or sys.version_info >= (3,7) else None) self.assertTrue(is_open_nul(fd)) - self.assertTrue(is_open_nul(fd + 1)) - os.close(fd + 1) + self.assertTrue(is_open_nul(fdn)) + os.close(fdn) self.assertTrue(is_open_nul(fd)) self.assertEqual(os.write(fd, b"1"), 1) @@ -87,7 +86,7 @@ def test_dup2(self): self.assertEqual(os.read(fd, 1), b"2") os.close(fd) - # fd+1 is already closed + # fdn is already closed os.close(fd + 2) os.unlink(test_filename) @@ -101,29 +100,33 @@ def test_dup(self): # The source must be valid. self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor", os.dup, -1) - self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor", os.dup, 99) + self.assertRaisesMessage(OSError, "[Errno 9] Bad file descriptor", os.dup, fd1 + 10000) # Test basic functionality. fd2 = os.dup(fd1) - self.assertTrue(fd2 == fd1 + 1) + if not (is_cli and is_posix): + # On IronPython/Posix, the first call to dup or dup2 may load Mono.Unix.dll and the corresponding `.so` + # This makes the fd numbers less predictable + self.assertEqual(fd2, fd1 + 1) self.assertTrue(is_open(fd2)) self.assertTrue(is_open(fd1)) os.close(fd1) - self.assertTrue(not is_open(fd1)) + self.assertFalse(is_open(fd1)) self.assertTrue(is_open(fd2)) - # dup uses the lowest-numbered unused descriptor for the new descriptor. fd3 = os.dup(fd2) - self.assertEqual(fd3, fd1) + # dup uses the lowest-numbered unused descriptor for the new descriptor. + if not (is_cli and is_posix): + self.assertEqual(fd3, fd1) - # writing though the duplicated fd writes to the same file - self.assertEqual(os.write(fd2, b"fd2"), 3) - self.assertEqual(os.write(fd3, b"fd3"), 3) - self.assertEqual(os.write(fd2, b"fd2-again"), 9) + # writing through the duplicated fd writes to the same file + self.assertEqual(os.write(fd2, b"(fd2)"), 5) + self.assertEqual(os.write(fd3, b"(=====fd3=====)"), 15) + self.assertEqual(os.write(fd2, b"(fd2-again)"), 11) os.close(fd3) self.assertEqual(os.lseek(fd2, 0, os.SEEK_SET), 0) - self.assertEqual(os.read(fd2, 15), b"fd2fd3fd2-again") + self.assertEqual(os.read(fd2, 5 + 15 + 11), b"(fd2)(=====fd3=====)(fd2-again)") # cleanup os.close(fd2) @@ -133,20 +136,20 @@ def test_dup_file(self): test_filename = "tmp_%d.dup-file.test" % os.getpid() file1 = open(test_filename, 'w+') - file1.write("file1") + file1.write("(file1)") file1.flush() fd2 = os.dup(file1.fileno()) file2 = open(fd2, 'w+') self.assertNotEqual(file1.fileno(), file2.fileno()) - file2.write("file2") + file2.write("(======file2======)") file2.flush() - file1.write("file1-again") + file1.write("(file1-again)") file1.close() file2.seek(0) - self.assertEqual(file2.read(), "file1file2file1-again") + self.assertEqual(file2.read(), "(file1)(======file2======)(file1-again)") file2.close() os.unlink(test_filename) diff --git a/Tests/test_file.py b/Tests/test_file.py index fea75c0dd..97cc97e36 100644 --- a/Tests/test_file.py +++ b/Tests/test_file.py @@ -783,10 +783,31 @@ def test_opener_uncallable(self): self.assertRaises(TypeError, open, "", "r", opener=uncallable_opener) + def test_open_wbplus(self): + with open(self.temp_file, "wb+") as f: + 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"abdef") + f.seek(0) + self.assertEqual(f.read(2), b"ab") + f.fileno() # does not move the file pointer + self.assertEqual(f.read(2), b"de") + f.write(b"z") + f.seek(0) + self.assertEqual(f.read(), b"abdez") + def test_open_abplus(self): with open(self.temp_file, "ab+") as f: f.write(b"abc") f.seek(0) - self.assertEqual(f.read(), b"abc") + 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") run_test(__name__) From 9358e4ba374d47949093bdedccd07d7ba172164e Mon Sep 17 00:00:00 2001 From: slozier Date: Thu, 12 Dec 2024 21:16:37 -0500 Subject: [PATCH 10/16] Improve performance in argument unpacking (#1838) * Cache parameters list * Use Queue to speed up popping --- .../Compiler/Ast/FunctionDefinition.cs | 4 +- .../Runtime/Binding/MetaPythonFunction.cs | 19 ++--- .../Runtime/Operations/PythonOps.cs | 75 ++++++++++++------- 3 files changed, 61 insertions(+), 37 deletions(-) diff --git a/Src/IronPython/Compiler/Ast/FunctionDefinition.cs b/Src/IronPython/Compiler/Ast/FunctionDefinition.cs index 36e74b0bb..bb5d18d35 100644 --- a/Src/IronPython/Compiler/Ast/FunctionDefinition.cs +++ b/Src/IronPython/Compiler/Ast/FunctionDefinition.cs @@ -79,7 +79,9 @@ internal override MSAst.Expression LocalContext { public IList Parameters => _parameters; - internal override string[] ParameterNames => ArrayUtils.ConvertAll(_parameters, val => val.Name); + private string[] _parameterNames = null; + + internal override string[] ParameterNames => _parameterNames ??= ArrayUtils.ConvertAll(_parameters, val => val.Name); internal override int ArgCount { get { diff --git a/Src/IronPython/Runtime/Binding/MetaPythonFunction.cs b/Src/IronPython/Runtime/Binding/MetaPythonFunction.cs index cd492f595..d2bc0044c 100644 --- a/Src/IronPython/Runtime/Binding/MetaPythonFunction.cs +++ b/Src/IronPython/Runtime/Binding/MetaPythonFunction.cs @@ -551,6 +551,7 @@ private bool TryFinishList(Expression[] exprArgs, List paramsArgs) { // make a single copy. exprArgs[_func.Value.ExpandListPosition] = Ast.Call( typeof(PythonOps).GetMethod(nameof(PythonOps.GetOrCopyParamsTuple)), + _codeContext ?? AstUtils.Constant(DefaultContext.Default), GetFunctionParam(), AstUtils.Convert(_userProvidedParams, typeof(object)) ); @@ -653,7 +654,7 @@ private void AddCheckForNoExtraParameters(Expression[] exprArgs) { Ast.Call( typeof(PythonOps).GetMethod(nameof(PythonOps.CheckParamsZero)), AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)), - _params + _params // Queue ) ); } else if (_userProvidedParams != null) { @@ -805,7 +806,7 @@ private Expression ExtractDefaultValue(string argName, int dfltIndex) { AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)), AstUtils.Constant(dfltIndex), AstUtils.Constant(argName, typeof(string)), - VariableOrNull(_params, typeof(PythonList)), + VariableOrNull(_params, typeof(Queue)), VariableOrNull(_dict, typeof(PythonDictionary)) ); } @@ -843,7 +844,7 @@ private Expression ExtractFromListOrDictionary(string name) { AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)), // function AstUtils.Constant(name, typeof(string)), // name _paramsLen, // arg count - _params, // params list + _params, // Queue AstUtils.Convert(_dict, typeof(IDictionary)) // dictionary ); } @@ -860,17 +861,13 @@ private void EnsureParams() { /// Helper function to extract the next argument from the params list. /// private Expression ExtractNextParamsArg() { - if (!_extractedParams) { - MakeParamsCopy(_userProvidedParams); - - _extractedParams = true; - } + EnsureParams(); return Ast.Call( typeof(PythonOps).GetMethod(nameof(PythonOps.ExtractParamsArgument)), AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)), // function AstUtils.Constant(Signature.ArgumentCount), // arg count - _params // list + _params // Queue ); } @@ -945,7 +942,7 @@ private static Expression VariableOrNull(ParameterExpression var, Type type) { private void MakeParamsCopy(Expression/*!*/ userList) { Debug.Assert(_params == null); - _temps.Add(_params = Ast.Variable(typeof(PythonList), "$list")); + _temps.Add(_params = Ast.Variable(typeof(Queue), "$list")); _temps.Add(_paramsLen = Ast.Variable(typeof(int), "$paramsLen")); EnsureInit(); @@ -965,7 +962,7 @@ private void MakeParamsCopy(Expression/*!*/ userList) { _init.Add( Ast.Assign(_paramsLen, Ast.Add( - Ast.Call(_params, typeof(PythonList).GetMethod(nameof(PythonList.__len__))), + Ast.Property(_params, nameof(Queue.Count)), AstUtils.Constant(Signature.GetProvidedPositionalArgumentCount()) ) ) diff --git a/Src/IronPython/Runtime/Operations/PythonOps.cs b/Src/IronPython/Runtime/Operations/PythonOps.cs index 380c5006f..2d4554f9b 100644 --- a/Src/IronPython/Runtime/Operations/PythonOps.cs +++ b/Src/IronPython/Runtime/Operations/PythonOps.cs @@ -938,7 +938,7 @@ internal static bool TryInvokeLengthHint(CodeContext context, object? sequence, return CallWithContext(context, func, args); } - [Obsolete("Use ObjectOpertaions instead")] + [Obsolete("Use ObjectOperations instead")] public static object? CallWithArgsTupleAndKeywordDictAndContext(CodeContext/*!*/ context, object func, object[] args, string[] names, object argsTuple, object kwDict) { IDictionary? kws = kwDict as IDictionary; if (kws == null && kwDict != null) throw PythonOps.TypeError("argument after ** must be a dictionary"); @@ -2634,11 +2634,26 @@ public static void VerifyUnduplicatedByName(PythonFunction function, string name } - public static PythonList CopyAndVerifyParamsList(CodeContext context, PythonFunction function, object list) { - return new PythonList(context, list); + [EditorBrowsable(EditorBrowsableState.Never)] + public static Queue CopyAndVerifyParamsList(CodeContext context, PythonFunction function, object list) { + if (list is not IEnumerable e) { + if (!TryGetEnumerator(context, list, out IEnumerator? enumerator)) { + // TODO: CPython 3.5 uses "an iterable" in the error message instead of "a sequence" + throw TypeError($"{function.__name__}() argument after * must be a sequence, not {PythonOps.GetPythonTypeName(list)}"); + } + e = IEnumerableFromEnumerator(enumerator); + } + return new Queue(e); + + static IEnumerable IEnumerableFromEnumerator(IEnumerator ie) { + while (ie.MoveNext()) { + yield return ie.Current; + } + } } - public static PythonTuple UserMappingToPythonTuple(CodeContext/*!*/ context, object list, string funcName) { + [EditorBrowsable(EditorBrowsableState.Never)] + public static PythonTuple UserMappingToPythonTuple(CodeContext/*!*/ context, object? list, string funcName) { if (!TryGetEnumeratorObject(context, list, out object? enumerator)) { // TODO: CPython 3.5 uses "an iterable" in the error message instead of "a sequence" throw TypeError($"{funcName}() argument after * must be a sequence, not {PythonOps.GetPythonTypeName(list)}"); @@ -2647,35 +2662,43 @@ public static PythonTuple UserMappingToPythonTuple(CodeContext/*!*/ context, obj return PythonTuple.Make(enumerator); } - public static PythonTuple GetOrCopyParamsTuple(PythonFunction function, object input) { - if (input == null) { - throw PythonOps.TypeError("{0}() argument after * must be a sequence, not NoneType", function.__name__); + [EditorBrowsable(EditorBrowsableState.Never)] + public static PythonTuple GetOrCopyParamsTuple(CodeContext/*!*/ context, PythonFunction function, object? input) { + if (input is PythonTuple t && t.GetType() == typeof(PythonTuple)) { + return t; } - - return PythonTuple.Make(input); + return UserMappingToPythonTuple(context, input, function.__name__); } - public static object? ExtractParamsArgument(PythonFunction function, int argCnt, PythonList list) { - if (list.__len__() != 0) { - return list.pop(0); + [EditorBrowsable(EditorBrowsableState.Never)] + public static object? ExtractParamsArgument(PythonFunction function, int argCnt, Queue list) { + if (list.Count != 0) { + return list.Dequeue(); } throw function.BadArgumentError(argCnt); } - public static void AddParamsArguments(PythonList list, params object[] args) { - for (int i = 0; i < args.Length; i++) { - list.insert(i, args[i]); + [EditorBrowsable(EditorBrowsableState.Never)] + public static void AddParamsArguments(Queue list, params object[] args) { + var len = list.Count; + foreach (var arg in args) { + list.Enqueue(arg); + } + // put existing arguments at the end + for (int i = 0; i < len; i++) { + list.Enqueue(list.Dequeue()); } } /// /// Extracts an argument from either the dictionary or params /// - public static object? ExtractAnyArgument(PythonFunction function, string name, int argCnt, PythonList list, IDictionary dict) { + [EditorBrowsable(EditorBrowsableState.Never)] + public static object? ExtractAnyArgument(PythonFunction function, string name, int argCnt, Queue list, IDictionary dict) { object? val; if (dict.Contains(name)) { - if (list.__len__() != 0) { + if (list.Count != 0) { throw MultipleKeywordArgumentError(function, name); } val = dict[name]; @@ -2683,8 +2706,8 @@ public static void AddParamsArguments(PythonList list, params object[] args) { return val; } - if (list.__len__() != 0) { - return list.pop(0); + if (list.Count != 0) { + return list.Dequeue(); } if (function.ExpandDictPosition == -1 && dict.Count > 0) { @@ -2726,9 +2749,10 @@ public static ArgumentTypeException SimpleTypeError(string message) { return function.Defaults[index]; } - public static object? GetFunctionParameterValue(PythonFunction function, int index, string name, PythonList? extraArgs, PythonDictionary? dict) { - if (extraArgs != null && extraArgs.__len__() > 0) { - return extraArgs.pop(0); + [EditorBrowsable(EditorBrowsableState.Never)] + public static object? GetFunctionParameterValue(PythonFunction function, int index, string name, Queue? extraArgs, PythonDictionary? dict) { + if (extraArgs != null && extraArgs.Count > 0) { + return extraArgs.Dequeue(); } if (dict != null && dict.TryRemoveValue(name, out object val)) { @@ -2746,9 +2770,10 @@ public static ArgumentTypeException SimpleTypeError(string message) { return function.__kwdefaults__?[name]; } - public static void CheckParamsZero(PythonFunction function, PythonList extraArgs) { - if (extraArgs.__len__() != 0) { - throw function.BadArgumentError(extraArgs.__len__() + function.NormalArgumentCount); + [EditorBrowsable(EditorBrowsableState.Never)] + public static void CheckParamsZero(PythonFunction function, Queue extraArgs) { + if (extraArgs.Count != 0) { + throw function.BadArgumentError(extraArgs.Count + function.NormalArgumentCount); } } From 05cfb0e3b21cb2b212b9c32c399dd5de366388fa Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Thu, 12 Dec 2024 20:48:10 -0800 Subject: [PATCH 11/16] Support mode `ab+` equivalent through os.open (#1839) * Use dual stream for os.open with O_APPEND * Maintain file position when accessing file descriptor * Add test * Fix typo --- Src/IronPython.Modules/nt.cs | 48 +++++++++++++-------- Src/IronPython/Modules/_fileio.cs | 13 ++++++ Src/IronPython/Runtime/PythonFileManager.cs | 10 ++--- Tests/modules/system_related/test_os.py | 24 +++++++++++ 4 files changed, 72 insertions(+), 23 deletions(-) diff --git a/Src/IronPython.Modules/nt.cs b/Src/IronPython.Modules/nt.cs index 8d91d5438..5ef569320 100644 --- a/Src/IronPython.Modules/nt.cs +++ b/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)); } } @@ -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; @@ -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); diff --git a/Src/IronPython/Modules/_fileio.cs b/Src/IronPython/Modules/_fileio.cs index 93e2135bf..700a6691a 100644 --- a/Src/IronPython/Modules/_fileio.cs +++ b/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); + // 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); diff --git a/Src/IronPython/Runtime/PythonFileManager.cs b/Src/IronPython/Runtime/PythonFileManager.cs index 64a81bb9b..c98811608 100644 --- a/Src/IronPython/Runtime/PythonFileManager.cs +++ b/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()), diff --git a/Tests/modules/system_related/test_os.py b/Tests/modules/system_related/test_os.py index 24784fd4a..d27c6dece 100644 --- a/Tests/modules/system_related/test_os.py +++ b/Tests/modules/system_related/test_os.py @@ -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") @@ -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__) From ddd1c696fd9d14d229ad7c07f250c3ca1fe61b55 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Fri, 13 Dec 2024 17:48:21 -0800 Subject: [PATCH 12/16] Use symbolic error codes with OSError (#1840) * Use symbolic error codes with OSError * Fix typos --- Src/IronPython.Modules/nt.cs | 10 +++---- Src/IronPython/Modules/_fileio.cs | 8 +++--- Src/IronPython/Modules/_io.cs | 4 ++- Src/IronPython/Runtime/PythonFileManager.cs | 30 ++++++++++++++++----- Src/Scripts/generate_os_codes.py | 13 +++++++++ 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/Src/IronPython.Modules/nt.cs b/Src/IronPython.Modules/nt.cs index 5ef569320..21e340e4e 100644 --- a/Src/IronPython.Modules/nt.cs +++ b/Src/IronPython.Modules/nt.cs @@ -378,7 +378,7 @@ public static int dup2(CodeContext/*!*/ context, int fd, int fd2) { } if (!fileManager.ValidateFdRange(fd2)) { - throw PythonOps.OSError(9, "Bad file descriptor"); + throw PythonOps.OSError(PythonErrorNumber.EBADF, "Bad file descriptor"); } if (fileManager.TryGetStreams(fd2, out _)) { @@ -455,7 +455,7 @@ public static object fstat(CodeContext/*!*/ context, int fd) { if (streams.IsStandardIOStream()) return new stat_result(0x1000); if (StatStream(streams.ReadStream) is not null and var res) return res; } - return LightExceptions.Throw(PythonOps.OSError(9, "Bad file descriptor")); + return LightExceptions.Throw(PythonOps.OSError(PythonErrorNumber.EBADF, "Bad file descriptor")); static object? StatStream(Stream stream) { if (stream is FileStream fs) return lstat(fs.Name, new Dictionary(1)); @@ -481,7 +481,7 @@ public static void fsync(CodeContext context, int fd) { try { streams.Flush(); } catch (IOException) { - throw PythonOps.OSError(9, "Bad file descriptor"); + throw PythonOps.OSError(PythonErrorNumber.EBADF, "Bad file descriptor"); } } @@ -988,7 +988,7 @@ public static Bytes read(CodeContext/*!*/ context, int fd, int buffersize) { try { PythonContext pythonContext = context.LanguageContext; var streams = pythonContext.FileManager.GetStreams(fd); - if (!streams.ReadStream.CanRead) throw PythonOps.OSError(9, "Bad file descriptor"); + if (!streams.ReadStream.CanRead) throw PythonOps.OSError(PythonErrorNumber.EBADF, "Bad file descriptor"); return Bytes.Make(streams.Read(buffersize)); } catch (Exception e) { @@ -1872,7 +1872,7 @@ public static int write(CodeContext/*!*/ context, int fd, [NotNone] IBufferProto using var buffer = data.GetBuffer(); PythonContext pythonContext = context.LanguageContext; var streams = pythonContext.FileManager.GetStreams(fd); - if (!streams.WriteStream.CanWrite) throw PythonOps.OSError(9, "Bad file descriptor"); + if (!streams.WriteStream.CanWrite) throw PythonOps.OSError(PythonErrorNumber.EBADF, "Bad file descriptor"); return streams.Write(buffer); } catch (Exception e) { diff --git a/Src/IronPython/Modules/_fileio.cs b/Src/IronPython/Modules/_fileio.cs index 700a6691a..5e093c6fb 100644 --- a/Src/IronPython/Modules/_fileio.cs +++ b/Src/IronPython/Modules/_fileio.cs @@ -149,7 +149,7 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo } if (!_context.FileManager.TryGetStreams(fd, out _streams)) { - throw PythonOps.OSError(9, "Bad file descriptor"); + throw PythonOps.OSError(PythonFileManager.EBADF, "Bad file descriptor"); } } else { throw PythonOps.TypeError("expected integer from opener"); @@ -504,13 +504,13 @@ private static void AddFilename(CodeContext context, string name, Exception ioe) } private static Stream OpenFile(CodeContext/*!*/ context, PlatformAdaptationLayer pal, string name, FileMode fileMode, FileAccess fileAccess, FileShare fileShare) { - if (string.IsNullOrWhiteSpace(name)) throw PythonOps.OSError(2, "No such file or directory", filename: name); + if (string.IsNullOrWhiteSpace(name)) throw PythonOps.OSError(PythonFileManager.ENOENT, "No such file or directory", filename: name); try { return pal.OpenFileStream(name, fileMode, fileAccess, fileShare, 1); // Use a 1 byte buffer size to disable buffering (if the FileStream implementation supports it). } catch (UnauthorizedAccessException) { - throw PythonOps.OSError(13, "Permission denied", name); + throw PythonOps.OSError(PythonFileManager.EACCES, "Permission denied", name); } catch (FileNotFoundException) { - throw PythonOps.OSError(2, "No such file or directory", name); + throw PythonOps.OSError(PythonFileManager.ENOENT, "No such file or directory", name); } catch (IOException e) { AddFilename(context, name, e); throw; diff --git a/Src/IronPython/Modules/_io.cs b/Src/IronPython/Modules/_io.cs index daaa3caa6..94cb80a30 100644 --- a/Src/IronPython/Modules/_io.cs +++ b/Src/IronPython/Modules/_io.cs @@ -31,7 +31,8 @@ public static partial class PythonIOModule { private static readonly object _blockingIOErrorKey = new object(); private static readonly object _unsupportedOperationKey = new object(); - // Values of the O_flags below has to be identical with flags defined in PythonNT + + // Values of the O_flags below have to be identical with flags defined in PythonNT #region Generated Common O_Flags @@ -57,6 +58,7 @@ public static partial class PythonIOModule { #endregion + [SpecialName] public static void PerformModuleReload(PythonContext/*!*/ context, PythonDictionary/*!*/ dict) { context.EnsureModuleException( diff --git a/Src/IronPython/Runtime/PythonFileManager.cs b/Src/IronPython/Runtime/PythonFileManager.cs index c98811608..03ee1d9b3 100644 --- a/Src/IronPython/Runtime/PythonFileManager.cs +++ b/Src/IronPython/Runtime/PythonFileManager.cs @@ -183,7 +183,7 @@ public void CloseStreams(PythonFileManager? manager) { /// /// /// PythonFileManager emulates a file descriptor table. On Windows, .NET uses Win32 API which uses file handles - /// rather than file descriptors. The emulation is necesary to support Python API, which in some places uses file descriptors. + /// rather than file descriptors. The emulation is necessary to support Python API, which in some places uses file descriptors. /// /// The manager maintains a mapping between open files (or system file-like objects) and a "descriptor", being a small non-negative integer. /// Unlike in CPython, the descriptors are allocated lazily, meaning they are allocated only when they become exposed (requested) @@ -198,7 +198,7 @@ public void CloseStreams(PythonFileManager? manager) { /// In such situations, only one of the FileIO may be opened with flag `closefd` (CPython rule). /// /// The second lever of sharing of open files is below the file descriptor level. A file descriptor can be duplicated using dup/dup2, - /// but the duplicated descriptor is still refering to the same open file in the filesystem. In such case, the manager maintains + /// but the duplicated descriptor is still referring to the same open file in the filesystem. In such case, the manager maintains /// a separate StreamBox for the duplicated descriptor, but the StreamBoxes for both descriptors share the underlying Streams. /// Both such descriptors have to be closed independently by the user code (either explicitly by os.close(fd) or through close() /// on the FileIO objects), but the underlying shared streams are closed only when all such duplicated descriptors are closed. @@ -211,6 +211,24 @@ internal class PythonFileManager { /// public const int LIMIT_OFILE = 0x100000; // hard limit on Linux + + // Values of the Errno codes below have to be identical with values defined in PythonErrorNumber + + #region Generated Common Errno Codes + + // *** BEGIN GENERATED CODE *** + // generated by function: generate_common_errno_codes from: generate_os_codes.py + + internal const int ENOENT = 2; + internal const int EBADF = 9; + internal const int EACCES = 13; + internal const int EMFILE = 24; + + // *** END GENERATED CODE *** + + #endregion + + private readonly object _synchObject = new(); private readonly Dictionary _table = new(); private const int _offset = 3; // number of lowest keys that are not automatically allocated @@ -218,14 +236,14 @@ internal class PythonFileManager { private readonly ConcurrentDictionary _refs = new(); // This version of Add is used with genuine file descriptors (Unix). - // Exception: support dup2 on all frameworks/platfroms. + // Exception: support dup2 on all frameworks/platforms. public int Add(int id, StreamBox streams) { ContractUtils.RequiresNotNull(streams, nameof(streams)); ContractUtils.Requires(streams.Id < 0, nameof(streams)); ContractUtils.Requires(id >= 0, nameof(id)); lock (_synchObject) { if (_table.ContainsKey(id)) { - throw PythonOps.OSError(9, "Bad file descriptor", id.ToString()); + throw PythonOps.OSError(EBADF, "Bad file descriptor", id.ToString()); } streams.Id = id; _table.Add(id, streams); @@ -243,7 +261,7 @@ public int Add(StreamBox streams) { while (_table.ContainsKey(_current)) { _current++; if (_current >= LIMIT_OFILE) - throw PythonOps.OSError(24, "Too many open files"); + throw PythonOps.OSError(EMFILE, "Too many open files"); } streams.Id = _current; _table.Add(_current, streams); @@ -280,7 +298,7 @@ public StreamBox GetStreams(int id) { if (TryGetStreams(id, out StreamBox? streams)) { return streams; } - throw PythonOps.OSError(9, "Bad file descriptor"); + throw PythonOps.OSError(EBADF, "Bad file descriptor"); } public int GetOrAssignId(StreamBox streams) { diff --git a/Src/Scripts/generate_os_codes.py b/Src/Scripts/generate_os_codes.py index 811ca0178..912923bd6 100644 --- a/Src/Scripts/generate_os_codes.py +++ b/Src/Scripts/generate_os_codes.py @@ -102,6 +102,18 @@ def darwin_code_expr(codes, fmt): def linux_code_expr(codes, fmt): return fmt(codes[linux_idx]) +common_errno_codes = ['ENOENT', 'EBADF', 'EACCES', 'EMFILE'] + +def generate_common_errno_codes(cw): + for name in common_errno_codes: + codes = errorvalues[name] + + value = windows_code_expr(codes, str) + if (all(c.isdigit() for c in value)): + cw.write(f"internal const int {name} = {value};") + else: + cw.write(f"internal static int {name} => {value};") + def generate_errno_names(cw): def is_windows_alias(name): return "WSA" + name in errorvalues and name in errorvalues and errorvalues["WSA" + name][windows_idx] == errorvalues[name][windows_idx] @@ -208,6 +220,7 @@ def generate_common_O_flags(cw): def main(): return generate( ("Errno Codes", generate_errno_codes), + ("Common Errno Codes", generate_common_errno_codes), ("Errno Names", generate_errno_names), ("O_Flags", generate_all_O_flags), ("Common O_Flags", generate_common_O_flags), From a0f828f5e896e6abe22ac74327ead27f2fede92c Mon Sep 17 00:00:00 2001 From: slozier Date: Sat, 14 Dec 2024 00:00:58 -0500 Subject: [PATCH 13/16] Bump dotnet tool to .NET 8 (#1834) * Bump dotnet tool to .NET 8 * Update runtimeconfig.template.json --- Package/dotnettool/IronPython.Console.csproj | 4 ++-- Package/dotnettool/runtimeconfig.template.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Package/dotnettool/IronPython.Console.csproj b/Package/dotnettool/IronPython.Console.csproj index 9fb8878c3..068b735ff 100644 --- a/Package/dotnettool/IronPython.Console.csproj +++ b/Package/dotnettool/IronPython.Console.csproj @@ -2,7 +2,7 @@ - net6.0 + net8.0 Exe ipy false @@ -40,7 +40,7 @@ This package contains a standalone Python interpreter, invokable from the comman - + diff --git a/Package/dotnettool/runtimeconfig.template.json b/Package/dotnettool/runtimeconfig.template.json index 6b5bfaaf5..e11bce72e 100644 --- a/Package/dotnettool/runtimeconfig.template.json +++ b/Package/dotnettool/runtimeconfig.template.json @@ -1,7 +1,7 @@ { "framework": { "name": "Microsoft.NETCore.App", - "version": "6.0.0" + "version": "8.0.0" }, "configProperties": { "Microsoft.NETCore.DotNetHostPolicy.SetAppPaths": true From 389a844a65db673a13cf0c9a8c0f50a73bc999d3 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Tue, 17 Dec 2024 18:37:53 -0800 Subject: [PATCH 14/16] Fix file seek errors (#1842) --- Src/IronPython/Modules/_fileio.cs | 42 ++++++++++++------- Src/IronPython/Runtime/PythonFileManager.cs | 1 + Src/Scripts/generate_os_codes.py | 2 +- Tests/test_file.py | 45 +++++++++++++++++++++ 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/Src/IronPython/Modules/_fileio.cs b/Src/IronPython/Modules/_fileio.cs index 5e093c6fb..51b1b58e8 100644 --- a/Src/IronPython/Modules/_fileio.cs +++ b/Src/IronPython/Modules/_fileio.cs @@ -133,7 +133,7 @@ public FileIO(CodeContext/*!*/ context, string name, string mode = "r", bool clo // 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+") { + if (this.mode.StartsWith("ab", StringComparison.InvariantCulture)) { _streams.WriteStream.Seek(0L, SeekOrigin.End); } if (!_streams.IsSingleStream) { @@ -372,26 +372,40 @@ public override BigInteger readinto(CodeContext/*!*/ context, object buf) { return readinto(bufferProtocol); } - [Documentation("seek(offset: int[, whence: int]) -> None. Move to new file position.\n\n" - + "Argument offset is a byte count. Optional argument whence defaults to\n" - + "0 (offset from start of file, offset should be >= 0); other values are 1\n" - + "(move relative to current position, positive or negative), and 2 (move\n" - + "relative to end of file, usually negative, although many platforms allow\n" - + "seeking beyond the end of a file).\n" - + "Note that not all file objects are seekable." - )] + + [Documentation(""" + seek(offset: int[, whence: int]) -> int. Change stream position. + + Argument offset is a byte count. Optional argument whence defaults to + 0 or `os.SEEK_SET` (offset from start of file, offset should be >= 0); + other values are 1 or `os.SEEK_CUR` (move relative to current position, + positive or negative), and 2 or `os.SEEK_END` (move relative to end of + file, usually negative, although many platforms allow seeking beyond + the end of a file, by adding zeros to enlarge the file). + + Return the new absolute position. + + Note that not all file objects are seekable. + """)] public override BigInteger seek(CodeContext/*!*/ context, BigInteger offset, [Optional] object whence) { _checkClosed(); - return _streams.ReadStream.Seek((long)offset, (SeekOrigin)GetInt(whence)); - } + var origin = (SeekOrigin)GetInt(whence); + if (origin < SeekOrigin.Begin || origin > SeekOrigin.End) + throw PythonOps.OSError(PythonFileManager.EINVAL, "Invalid argument"); - public BigInteger seek(double offset, [Optional] object whence) { - _checkClosed(); + long ofs = checked((long)offset); + if (ofs < 0 && ClrModule.IsMono && origin == SeekOrigin.Current) { + // Mono does not support negative offsets with SeekOrigin.Current + // so we need to calculate the absolute offset + ofs += _streams.ReadStream.Position; + origin = SeekOrigin.Begin; + } - throw PythonOps.TypeError("an integer is required"); + return _streams.ReadStream.Seek(ofs, origin); } + [Documentation("seekable() -> bool. True if file supports random-access.")] public override bool seekable(CodeContext/*!*/ context) { _checkClosed(); diff --git a/Src/IronPython/Runtime/PythonFileManager.cs b/Src/IronPython/Runtime/PythonFileManager.cs index 03ee1d9b3..ca4dbef44 100644 --- a/Src/IronPython/Runtime/PythonFileManager.cs +++ b/Src/IronPython/Runtime/PythonFileManager.cs @@ -222,6 +222,7 @@ internal class PythonFileManager { internal const int ENOENT = 2; internal const int EBADF = 9; internal const int EACCES = 13; + internal const int EINVAL = 22; internal const int EMFILE = 24; // *** END GENERATED CODE *** diff --git a/Src/Scripts/generate_os_codes.py b/Src/Scripts/generate_os_codes.py index 912923bd6..b5b226581 100644 --- a/Src/Scripts/generate_os_codes.py +++ b/Src/Scripts/generate_os_codes.py @@ -102,7 +102,7 @@ def darwin_code_expr(codes, fmt): def linux_code_expr(codes, fmt): return fmt(codes[linux_idx]) -common_errno_codes = ['ENOENT', 'EBADF', 'EACCES', 'EMFILE'] +common_errno_codes = ['ENOENT', 'EBADF', 'EACCES', 'EINVAL', 'EMFILE'] def generate_common_errno_codes(cw): for name in common_errno_codes: diff --git a/Tests/test_file.py b/Tests/test_file.py index 97cc97e36..c40936dfc 100644 --- a/Tests/test_file.py +++ b/Tests/test_file.py @@ -783,6 +783,51 @@ def test_opener_uncallable(self): self.assertRaises(TypeError, open, "", "r", opener=uncallable_opener) + + def test_seek(self): + with open(self.temp_file, "w") as f: + f.write("abc") + self.assertRaises(TypeError, f.buffer.seek, 1.5) + if is_cli: + self.assertRaises(TypeError, f.seek, 1.5) # surprisingly, this doesn't raise an error in CPython + + with open(self.temp_file, "rb") as f: + self.assertEqual(f.tell(), 0) + self.assertEqual(f.read(1), b"a") + self.assertEqual(f.tell(), 1) + + f.seek(2) + self.assertEqual(f.tell(), 2) + self.assertEqual(f.read(1), b"c") + self.assertEqual(f.tell(), 3) + + f.seek(0, os.SEEK_SET) + self.assertEqual(f.tell(), 0) + self.assertEqual(f.read(1), b"a") + self.assertEqual(f.tell(), 1) + + f.seek(0, os.SEEK_CUR) + self.assertEqual(f.tell(), 1) + self.assertEqual(f.read(1), b"b") + self.assertEqual(f.tell(), 2) + + f.raw.seek(2, os.SEEK_SET) + f.raw.seek(-2, os.SEEK_CUR) + self.assertEqual(f.raw.tell(), 0) + self.assertEqual(f.raw.read(1), b"a") + self.assertEqual(f.raw.tell(), 1) + + f.raw.seek(-1, os.SEEK_END) + self.assertEqual(f.raw.tell(), 2) + self.assertEqual(f.raw.read(1), b"c") + self.assertEqual(f.raw.tell(), 3) + + self.assertRaises(TypeError, f.seek, 1.5) + self.assertRaises(TypeError, f.raw.seek, 1.5) + self.assertRaises(OSError, f.raw.seek, -1) + self.assertRaises(OSError, f.raw.seek, 0, -1) + + def test_open_wbplus(self): with open(self.temp_file, "wb+") as f: f.write(b"abc") From 93c945b75c2bdbf6c71ba4cd96e4fd51c6047d62 Mon Sep 17 00:00:00 2001 From: slozier Date: Wed, 18 Dec 2024 12:40:32 -0500 Subject: [PATCH 15/16] Adjust math.degrees to match CPython (#1844) --- Src/IronPython.Modules/math.cs | 4 +++- Tests/modules/misc/test_math.py | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Src/IronPython.Modules/math.cs b/Src/IronPython.Modules/math.cs index a7f3910a5..f104210b2 100644 --- a/Src/IronPython.Modules/math.cs +++ b/Src/IronPython.Modules/math.cs @@ -30,10 +30,12 @@ public static partial class PythonMath { public const double e = Math.E; private const double degreesToRadians = Math.PI / 180.0; + private const double radiansToDegrees = 180.0 / Math.PI; + private const int Bias = 0x3FE; public static double degrees(double radians) { - return Check(radians, radians / degreesToRadians); + return Check(radians, radians * radiansToDegrees); } public static double radians(double degrees) { diff --git a/Tests/modules/misc/test_math.py b/Tests/modules/misc/test_math.py index 1eb791932..718abb1e1 100644 --- a/Tests/modules/misc/test_math.py +++ b/Tests/modules/misc/test_math.py @@ -718,4 +718,10 @@ def test_integer_ratio(self): for flt, res in int_ratio_tests: self.assertEqual(flt.as_integer_ratio(), res) + def test_degrees(self): + # check that IronPython is doing the same conversion as CPython + self.assertNotEqual(0.06825994771674652 / (math.pi / 180), 0.06825994771674652 * (180 / math.pi)) + self.assertEqual(0.06825994771674652 * (180 / math.pi), 3.911006913953236) + self.assertEqual(math.degrees(0.06825994771674652), 3.911006913953236) + run_test(__name__) From f5bb69b2d4c613e92a457cf474c0ce6b51c170a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Thu, 19 Dec 2024 15:48:23 -0500 Subject: [PATCH 16/16] Version 3.4.2 --- CurrentVersion.props | 2 +- Package/nuget/IronPython.nuspec | 8 ++++---- Src/DLR | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CurrentVersion.props b/CurrentVersion.props index 1a5c8e936..bbd6bb924 100644 --- a/CurrentVersion.props +++ b/CurrentVersion.props @@ -3,7 +3,7 @@ 3 4 - 1 + 2 final 0 diff --git a/Package/nuget/IronPython.nuspec b/Package/nuget/IronPython.nuspec index 8711f555a..d2334abc2 100644 --- a/Package/nuget/IronPython.nuspec +++ b/Package/nuget/IronPython.nuspec @@ -19,22 +19,22 @@ This package contains the IronPython interpreter engine. ironpython python dynamic dlr - + - + - + - + diff --git a/Src/DLR b/Src/DLR index fe26cf45d..7e887d984 160000 --- a/Src/DLR +++ b/Src/DLR @@ -1 +1 @@ -Subproject commit fe26cf45d383be65a56f0d4b964644eff89e01a8 +Subproject commit 7e887d9848dbfeca268814aeb237c83dc1244c8a