[Python-checkins] bpo-40915: Fix mmap resize bugs on Windows (GH-29213)
tjguk
webhook-mailer at python.org
Tue Oct 26 17:56:52 EDT 2021
https://github.com/python/cpython/commit/aea5ecc458084e01534ea6a11f4181f369869082
commit: aea5ecc458084e01534ea6a11f4181f369869082
branch: main
author: Tim Golden <mail at timgolden.me.uk>
committer: tjguk <mail at timgolden.me.uk>
date: 2021-10-26T22:56:43+01:00
summary:
bpo-40915: Fix mmap resize bugs on Windows (GH-29213)
(original patch by eryksun)
Correctly hand various failure modes when resizing an mmap on Windows:
* Resizing a pagefile-backed mmap now creates a new mmap and copies data
* Attempting to resize when another mapping is held on the same file raises an OSError
* Attempting to resize a nametagged mmap raises an OSError if another mapping is held with the same nametag
files:
M Doc/library/mmap.rst
M Lib/test/test_mmap.py
M Modules/mmapmodule.c
diff --git a/Doc/library/mmap.rst b/Doc/library/mmap.rst
index da174753361b2..c1ebd80abd977 100644
--- a/Doc/library/mmap.rst
+++ b/Doc/library/mmap.rst
@@ -256,6 +256,14 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
with :const:`ACCESS_READ` or :const:`ACCESS_COPY`, resizing the map will
raise a :exc:`TypeError` exception.
+ **On Windows**: Resizing the map will raise an :exc:`OSError` if there are other
+ maps against the same named file. Resizing an anonymous map (ie against the
+ pagefile) will silently create a new map with the original data copied over
+ up to the length of the new size.
+
+ .. versionchanged:: 3.11
+ Correctly fails if attempting to resize when another map is held
+ Allows resize against an anonymous map on Windows
.. method:: rfind(sub[, start[, end]])
diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py
index 8f34c182f82ea..82e2d2adb7dcf 100644
--- a/Lib/test/test_mmap.py
+++ b/Lib/test/test_mmap.py
@@ -5,6 +5,7 @@
import os
import re
import itertools
+import random
import socket
import sys
import weakref
@@ -707,7 +708,6 @@ def test_write_returning_the_number_of_bytes_written(self):
self.assertEqual(mm.write(b"yz"), 2)
self.assertEqual(mm.write(b"python"), 6)
- @unittest.skipIf(os.name == 'nt', 'cannot resize anonymous mmaps on Windows')
def test_resize_past_pos(self):
m = mmap.mmap(-1, 8192)
self.addCleanup(m.close)
@@ -796,6 +796,80 @@ def test_madvise(self):
self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, 2), None)
self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, size), None)
+ @unittest.skipUnless(os.name == 'nt', 'requires Windows')
+ def test_resize_up_when_mapped_to_pagefile(self):
+ """If the mmap is backed by the pagefile ensure a resize up can happen
+ and that the original data is still in place
+ """
+ start_size = PAGESIZE
+ new_size = 2 * start_size
+ data = bytes(random.getrandbits(8) for _ in range(start_size))
+
+ m = mmap.mmap(-1, start_size)
+ m[:] = data
+ m.resize(new_size)
+ self.assertEqual(len(m), new_size)
+ self.assertEqual(m[:start_size], data[:start_size])
+
+ @unittest.skipUnless(os.name == 'nt', 'requires Windows')
+ def test_resize_down_when_mapped_to_pagefile(self):
+ """If the mmap is backed by the pagefile ensure a resize down up can happen
+ and that a truncated form of the original data is still in place
+ """
+ start_size = PAGESIZE
+ new_size = start_size // 2
+ data = bytes(random.getrandbits(8) for _ in range(start_size))
+
+ m = mmap.mmap(-1, start_size)
+ m[:] = data
+ m.resize(new_size)
+ self.assertEqual(len(m), new_size)
+ self.assertEqual(m[:new_size], data[:new_size])
+
+ @unittest.skipUnless(os.name == 'nt', 'requires Windows')
+ def test_resize_fails_if_mapping_held_elsewhere(self):
+ """If more than one mapping is held against a named file on Windows, neither
+ mapping can be resized
+ """
+ start_size = 2 * PAGESIZE
+ reduced_size = PAGESIZE
+
+ f = open(TESTFN, 'wb+')
+ f.truncate(start_size)
+ try:
+ m1 = mmap.mmap(f.fileno(), start_size)
+ m2 = mmap.mmap(f.fileno(), start_size)
+ with self.assertRaises(OSError):
+ m1.resize(reduced_size)
+ with self.assertRaises(OSError):
+ m2.resize(reduced_size)
+ m2.close()
+ m1.resize(reduced_size)
+ self.assertEqual(m1.size(), reduced_size)
+ self.assertEqual(os.stat(f.fileno()).st_size, reduced_size)
+ finally:
+ f.close()
+
+ @unittest.skipUnless(os.name == 'nt', 'requires Windows')
+ def test_resize_succeeds_with_error_for_second_named_mapping(self):
+ """If a more than one mapping exists of the same name, none of them can
+ be resized: they'll raise an Exception and leave the original mapping intact
+ """
+ start_size = 2 * PAGESIZE
+ reduced_size = PAGESIZE
+ tagname = "TEST"
+ data_length = 8
+ data = bytes(random.getrandbits(8) for _ in range(data_length))
+
+ m1 = mmap.mmap(-1, start_size, tagname=tagname)
+ m2 = mmap.mmap(-1, start_size, tagname=tagname)
+ m1[:data_length] = data
+ self.assertEqual(m2[:data_length], data)
+ with self.assertRaises(OSError):
+ m1.resize(reduced_size)
+ self.assertEqual(m1.size(), start_size)
+ self.assertEqual(m1[:data_length], data)
+ self.assertEqual(m2[:data_length], data)
class LargeMmapTests(unittest.TestCase):
diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c
index 6397b0d4b8109..dfa10f633bbd5 100644
--- a/Modules/mmapmodule.c
+++ b/Modules/mmapmodule.c
@@ -32,6 +32,7 @@
#ifdef MS_WINDOWS
#include <windows.h>
+#include <winternl.h>
static int
my_getpagesize(void)
{
@@ -376,14 +377,15 @@ is_resizeable(mmap_object *self)
{
if (self->exports > 0) {
PyErr_SetString(PyExc_BufferError,
- "mmap can't resize with extant buffers exported.");
+ "mmap can't resize with extant buffers exported.");
return 0;
}
if ((self->access == ACCESS_WRITE) || (self->access == ACCESS_DEFAULT))
return 1;
PyErr_Format(PyExc_TypeError,
- "mmap can't resize a readonly or copy-on-write memory map.");
+ "mmap can't resize a readonly or copy-on-write memory map.");
return 0;
+
}
@@ -503,51 +505,110 @@ mmap_resize_method(mmap_object *self,
}
{
+ /*
+ To resize an mmap on Windows:
+
+ - Close the existing mapping
+ - If the mapping is backed to a named file:
+ unmap the view, clear the data, and resize the file
+ If the file can't be resized (eg because it has other mapped references
+ to it) then let the mapping be recreated at the original size and set
+ an error code so an exception will be raised.
+ - Create a new mapping of the relevant size to the same file
+ - Map a new view of the resized file
+ - If the mapping is backed by the pagefile:
+ copy any previous data into the new mapped area
+ unmap the original view which will release the memory
+ */
#ifdef MS_WINDOWS
- DWORD dwErrCode = 0;
- DWORD off_hi, off_lo, newSizeLow, newSizeHigh;
- /* First, unmap the file view */
- UnmapViewOfFile(self->data);
- self->data = NULL;
- /* Close the mapping object */
+ DWORD error = 0, file_resize_error = 0;
+ char* old_data = self->data;
+ LARGE_INTEGER offset, max_size;
+ offset.QuadPart = self->offset;
+ max_size.QuadPart = self->offset + new_size;
+ /* close the file mapping */
CloseHandle(self->map_handle);
- self->map_handle = NULL;
- /* Move to the desired EOF position */
- newSizeHigh = (DWORD)((self->offset + new_size) >> 32);
- newSizeLow = (DWORD)((self->offset + new_size) & 0xFFFFFFFF);
- off_hi = (DWORD)(self->offset >> 32);
- off_lo = (DWORD)(self->offset & 0xFFFFFFFF);
- SetFilePointer(self->file_handle,
- newSizeLow, &newSizeHigh, FILE_BEGIN);
- /* Change the size of the file */
- SetEndOfFile(self->file_handle);
- /* Create another mapping object and remap the file view */
+ /* if the file mapping still exists, it cannot be resized. */
+ if (self->tagname) {
+ self->map_handle = OpenFileMapping(FILE_MAP_WRITE, FALSE,
+ self->tagname);
+ if (self->map_handle) {
+ PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE);
+ return NULL;
+ }
+ } else {
+ self->map_handle = NULL;
+ }
+
+ /* if it's not the paging file, unmap the view and resize the file */
+ if (self->file_handle != INVALID_HANDLE_VALUE) {
+ if (!UnmapViewOfFile(self->data)) {
+ return PyErr_SetFromWindowsErr(GetLastError());
+ };
+ self->data = NULL;
+ /* resize the file */
+ if (!SetFilePointerEx(self->file_handle, max_size, NULL,
+ FILE_BEGIN) ||
+ !SetEndOfFile(self->file_handle)) {
+ /* resizing failed. try to remap the file */
+ file_resize_error = GetLastError();
+ new_size = max_size.QuadPart = self->size;
+ }
+ }
+
+ /* create a new file mapping and map a new view */
+ /* FIXME: call CreateFileMappingW with wchar_t tagname */
self->map_handle = CreateFileMapping(
self->file_handle,
NULL,
PAGE_READWRITE,
- 0,
- 0,
+ max_size.HighPart,
+ max_size.LowPart,
self->tagname);
- if (self->map_handle != NULL) {
- self->data = (char *) MapViewOfFile(self->map_handle,
- FILE_MAP_WRITE,
- off_hi,
- off_lo,
- new_size);
+
+ error = GetLastError();
+ if (error == ERROR_ALREADY_EXISTS) {
+ CloseHandle(self->map_handle);
+ self->map_handle = NULL;
+ }
+ else if (self->map_handle != NULL) {
+ self->data = MapViewOfFile(self->map_handle,
+ FILE_MAP_WRITE,
+ offset.HighPart,
+ offset.LowPart,
+ new_size);
if (self->data != NULL) {
+ /* copy the old view if using the paging file */
+ if (self->file_handle == INVALID_HANDLE_VALUE) {
+ memcpy(self->data, old_data,
+ self->size < new_size ? self->size : new_size);
+ if (!UnmapViewOfFile(old_data)) {
+ error = GetLastError();
+ }
+ }
self->size = new_size;
- Py_RETURN_NONE;
- } else {
- dwErrCode = GetLastError();
+ }
+ else {
+ error = GetLastError();
CloseHandle(self->map_handle);
self->map_handle = NULL;
}
- } else {
- dwErrCode = GetLastError();
}
- PyErr_SetFromWindowsErr(dwErrCode);
- return NULL;
+
+ if (error) {
+ return PyErr_SetFromWindowsErr(error);
+ return NULL;
+ }
+ /* It's possible for a resize to fail, typically because another mapping
+ is still held against the same underlying file. Even if nothing has
+ failed -- ie we're still returning a valid file mapping -- raise the
+ error as an exception as the resize won't have happened
+ */
+ if (file_resize_error) {
+ PyErr_SetFromWindowsErr(file_resize_error);
+ return NULL;
+ }
+ Py_RETURN_NONE;
#endif /* MS_WINDOWS */
#ifdef UNIX
More information about the Python-checkins
mailing list