From 93425d5515e3c2747f837c7476b19e63a2714e5f Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Mon, 13 Jan 2025 20:36:06 -0800 Subject: [PATCH 1/3] Fix mmap.resize errors --- Src/IronPython.Modules/mmap.cs | 2 +- Tests/modules/io_related/test_mmap.py | 100 ++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 Tests/modules/io_related/test_mmap.py diff --git a/Src/IronPython.Modules/mmap.cs b/Src/IronPython.Modules/mmap.cs index 942f35a7d..7e915de5a 100644 --- a/Src/IronPython.Modules/mmap.cs +++ b/Src/IronPython.Modules/mmap.cs @@ -846,7 +846,7 @@ public void resize(long newsize) { if (newsize == 0) { // resizing to an empty mapped region is not allowed - throw WindowsError(_offset == 0 + throw WindowsError(_offset != 0 ? PythonExceptions._OSError.ERROR_ACCESS_DENIED : PythonExceptions._OSError.ERROR_FILE_INVALID ); diff --git a/Tests/modules/io_related/test_mmap.py b/Tests/modules/io_related/test_mmap.py new file mode 100644 index 000000000..9e0833540 --- /dev/null +++ b/Tests/modules/io_related/test_mmap.py @@ -0,0 +1,100 @@ +# Licensed to the .NET Foundation under one or more agreements. +# 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. + +''' +Tests the _mmap standard module. +''' + +import sys +import os +import errno +import mmap + +from iptest import IronPythonTestCase, is_cli, is_posix, is_windows, run_test + +class MmapTest(IronPythonTestCase): + + def setUp(self): + super(MmapTest, self).setUp() + + self.temp_file = os.path.join(self.temporary_dir, "temp_%d.dat" % os.getpid()) + + def tearDown(self): + self.delete_files(self.temp_file) + return super().tearDown() + + + def test_constants(self): + self.assertTrue(hasattr(mmap, "PAGESIZE")) + self.assertTrue(hasattr(mmap, "ALLOCATIONGRANULARITY")) + + self.assertEqual(mmap.ACCESS_READ, 1) + self.assertEqual(mmap.ACCESS_WRITE, 2) + self.assertEqual(mmap.ACCESS_COPY, 3) + if sys.version_info >= (3, 7) or is_cli: + self.assertEqual(mmap.ACCESS_DEFAULT, 0) + self.assertFalse(hasattr(mmap, "ACCESS_NONE")) + + if is_posix: + self.assertEqual(mmap.MAP_SHARED, 1) + self.assertEqual(mmap.MAP_PRIVATE, 2) + self.assertEqual(mmap.PROT_READ, 1) + self.assertEqual(mmap.PROT_WRITE, 2) + self.assertEqual(mmap.PROT_EXEC, 4) + + + def test_resize_errors(self): + with open(self.temp_file, "wb+") as f: + f.write(b"x" * mmap.ALLOCATIONGRANULARITY * 2) + + with open(self.temp_file, "rb+") as f: + m = mmap.mmap(f.fileno(), 0, offset=0) + with self.assertRaises(OSError) as cm: + m.resize(0) + + self.assertEqual(cm.exception.errno, errno.EINVAL) # 22 + if is_windows: + self.assertEqual(cm.exception.winerror, 1006) # ERROR_FILE_INVALID + self.assertEqual(cm.exception.strerror, "The volume for a file has been externally altered so that the opened file is no longer valid") + else: + self.assertEqual(cm.exception.strerror, "Invalid argument") + + + def test_resize_errors_negative(self): + with open(self.temp_file, "wb+") as f: + f.write(b"x" * mmap.ALLOCATIONGRANULARITY * 2) + + with open(self.temp_file, "rb+") as f: + m = mmap.mmap(f.fileno(), 0, offset=0) + if is_cli or sys.version_info >= (3, 5): + self.assertRaises(ValueError, m.resize, -1) + else: + self.assertRaises(OSError, m.resize, -1) + + m.close() + + + def test_resize_errors_offset(self): + with open(self.temp_file, "wb+") as f: + f.write(b"x" * mmap.ALLOCATIONGRANULARITY * 2) + + with open(self.temp_file, "rb+") as f: + m = mmap.mmap(f.fileno(), 0, offset=mmap.ALLOCATIONGRANULARITY) + + if is_windows: + with self.assertRaises(PermissionError) as cm: + m.resize(0) + self.assertEqual(cm.exception.errno, errno.EACCES) # 13 + self.assertEqual(cm.exception.winerror, 5) # ERROR_ACCESS_DENIED + self.assertEqual(cm.exception.strerror, "Access is denied") + else: + with self.assertRaises(OSError) as cm: + m.resize(0) + self.assertEqual(cm.exception.errno, errno.EINVAL) # 22 + self.assertEqual(cm.exception.strerror, "Invalid argument") + m.close() + + +run_test(__name__) + From 569bb1f63d4ea8c1655ea5ba8e59c4b8c628f6af Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Tue, 14 Jan 2025 12:43:01 -0800 Subject: [PATCH 2/3] Fix error on Mono --- Src/IronPython.Modules/mmap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/IronPython.Modules/mmap.cs b/Src/IronPython.Modules/mmap.cs index 7e915de5a..b1474ebe5 100644 --- a/Src/IronPython.Modules/mmap.cs +++ b/Src/IronPython.Modules/mmap.cs @@ -846,7 +846,7 @@ public void resize(long newsize) { if (newsize == 0) { // resizing to an empty mapped region is not allowed - throw WindowsError(_offset != 0 + throw WindowsError(_offset != 0 && RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? PythonExceptions._OSError.ERROR_ACCESS_DENIED : PythonExceptions._OSError.ERROR_FILE_INVALID ); From a641016495edd3a2b5911b302d6f3c5dee4419f6 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Tue, 14 Jan 2025 18:36:01 -0800 Subject: [PATCH 3/3] Fix mmap closing --- Src/IronPython.Modules/mmap.cs | 24 ++++++++++++++---------- Tests/modules/io_related/test_mmap.py | 9 +++++++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Src/IronPython.Modules/mmap.cs b/Src/IronPython.Modules/mmap.cs index b1474ebe5..b6cbb871e 100644 --- a/Src/IronPython.Modules/mmap.cs +++ b/Src/IronPython.Modules/mmap.cs @@ -812,6 +812,10 @@ public void resize(long newsize) { if (_handle.IsInvalid) { throw PythonNT.GetOsError(PythonErrno.EBADF); } + if (_view.Capacity == newsize) { + // resizing to the same size + return; + } if (newsize == 0) { // resizing to an empty mapped region is not allowed throw PythonNT.GetOsError(PythonErrno.EINVAL); @@ -844,14 +848,6 @@ public void resize(long newsize) { throw WindowsError(PythonExceptions._OSError.ERROR_INVALID_PARAMETER); } - if (newsize == 0) { - // resizing to an empty mapped region is not allowed - throw WindowsError(_offset != 0 && RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? PythonExceptions._OSError.ERROR_ACCESS_DENIED - : PythonExceptions._OSError.ERROR_FILE_INVALID - ); - } - if (_view.Capacity == newsize) { // resizing to the same size return; @@ -860,6 +856,14 @@ public void resize(long newsize) { long capacity = checked(_offset + newsize); try { + if (newsize == 0) { + // resizing to an empty mapped region is not allowed + throw WindowsError(_offset != 0 && RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + ? PythonExceptions._OSError.ERROR_ACCESS_DENIED + : PythonExceptions._OSError.ERROR_FILE_INVALID + ); + } + _view.Flush(); _view.Dispose(); _file.Dispose(); @@ -1143,13 +1147,13 @@ private void EnsureOpen() { } } - private struct MmapLocker : IDisposable { + private readonly struct MmapLocker : IDisposable { private readonly MmapDefault _mmap; public MmapLocker(MmapDefault mmap) { _mmap = mmap; - Interlocked.Increment(ref _mmap._refCount); _mmap.EnsureOpen(); + Interlocked.Increment(ref _mmap._refCount); } #region IDisposable Members diff --git a/Tests/modules/io_related/test_mmap.py b/Tests/modules/io_related/test_mmap.py index 9e0833540..249c7a955 100644 --- a/Tests/modules/io_related/test_mmap.py +++ b/Tests/modules/io_related/test_mmap.py @@ -18,7 +18,7 @@ class MmapTest(IronPythonTestCase): def setUp(self): super(MmapTest, self).setUp() - self.temp_file = os.path.join(self.temporary_dir, "temp_%d.dat" % os.getpid()) + self.temp_file = os.path.join(self.temporary_dir, "temp_mmap_%d.dat" % os.getpid()) def tearDown(self): self.delete_files(self.temp_file) @@ -60,6 +60,8 @@ def test_resize_errors(self): else: self.assertEqual(cm.exception.strerror, "Invalid argument") + self.assertTrue(m.closed) + def test_resize_errors_negative(self): with open(self.temp_file, "wb+") as f: @@ -69,8 +71,10 @@ def test_resize_errors_negative(self): m = mmap.mmap(f.fileno(), 0, offset=0) if is_cli or sys.version_info >= (3, 5): self.assertRaises(ValueError, m.resize, -1) + self.assertFalse(m.closed) else: self.assertRaises(OSError, m.resize, -1) + self.assertTrue(m.closed) m.close() @@ -93,7 +97,8 @@ def test_resize_errors_offset(self): m.resize(0) self.assertEqual(cm.exception.errno, errno.EINVAL) # 22 self.assertEqual(cm.exception.strerror, "Invalid argument") - m.close() + + self.assertTrue(m.closed) run_test(__name__)