Skip to content

Commit aea5ecc

Browse files
authored
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
1 parent b5ee794 commit aea5ecc

File tree

3 files changed

+178
-35
lines changed

3 files changed

+178
-35
lines changed

Doc/library/mmap.rst

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,14 @@ To map anonymous memory, -1 should be passed as the fileno along with the length
256256
with :const:`ACCESS_READ` or :const:`ACCESS_COPY`, resizing the map will
257257
raise a :exc:`TypeError` exception.
258258

259+
**On Windows**: Resizing the map will raise an :exc:`OSError` if there are other
260+
maps against the same named file. Resizing an anonymous map (ie against the
261+
pagefile) will silently create a new map with the original data copied over
262+
up to the length of the new size.
263+
264+
.. versionchanged:: 3.11
265+
Correctly fails if attempting to resize when another map is held
266+
Allows resize against an anonymous map on Windows
259267

260268
.. method:: rfind(sub[, start[, end]])
261269

Lib/test/test_mmap.py

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import os
66
import re
77
import itertools
8+
import random
89
import socket
910
import sys
1011
import weakref
@@ -707,7 +708,6 @@ def test_write_returning_the_number_of_bytes_written(self):
707708
self.assertEqual(mm.write(b"yz"), 2)
708709
self.assertEqual(mm.write(b"python"), 6)
709710

710-
@unittest.skipIf(os.name == 'nt', 'cannot resize anonymous mmaps on Windows')
711711
def test_resize_past_pos(self):
712712
m = mmap.mmap(-1, 8192)
713713
self.addCleanup(m.close)
@@ -796,6 +796,80 @@ def test_madvise(self):
796796
self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, 2), None)
797797
self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, size), None)
798798

799+
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
800+
def test_resize_up_when_mapped_to_pagefile(self):
801+
"""If the mmap is backed by the pagefile ensure a resize up can happen
802+
and that the original data is still in place
803+
"""
804+
start_size = PAGESIZE
805+
new_size = 2 * start_size
806+
data = bytes(random.getrandbits(8) for _ in range(start_size))
807+
808+
m = mmap.mmap(-1, start_size)
809+
m[:] = data
810+
m.resize(new_size)
811+
self.assertEqual(len(m), new_size)
812+
self.assertEqual(m[:start_size], data[:start_size])
813+
814+
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
815+
def test_resize_down_when_mapped_to_pagefile(self):
816+
"""If the mmap is backed by the pagefile ensure a resize down up can happen
817+
and that a truncated form of the original data is still in place
818+
"""
819+
start_size = PAGESIZE
820+
new_size = start_size // 2
821+
data = bytes(random.getrandbits(8) for _ in range(start_size))
822+
823+
m = mmap.mmap(-1, start_size)
824+
m[:] = data
825+
m.resize(new_size)
826+
self.assertEqual(len(m), new_size)
827+
self.assertEqual(m[:new_size], data[:new_size])
828+
829+
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
830+
def test_resize_fails_if_mapping_held_elsewhere(self):
831+
"""If more than one mapping is held against a named file on Windows, neither
832+
mapping can be resized
833+
"""
834+
start_size = 2 * PAGESIZE
835+
reduced_size = PAGESIZE
836+
837+
f = open(TESTFN, 'wb+')
838+
f.truncate(start_size)
839+
try:
840+
m1 = mmap.mmap(f.fileno(), start_size)
841+
m2 = mmap.mmap(f.fileno(), start_size)
842+
with self.assertRaises(OSError):
843+
m1.resize(reduced_size)
844+
with self.assertRaises(OSError):
845+
m2.resize(reduced_size)
846+
m2.close()
847+
m1.resize(reduced_size)
848+
self.assertEqual(m1.size(), reduced_size)
849+
self.assertEqual(os.stat(f.fileno()).st_size, reduced_size)
850+
finally:
851+
f.close()
852+
853+
@unittest.skipUnless(os.name == 'nt', 'requires Windows')
854+
def test_resize_succeeds_with_error_for_second_named_mapping(self):
855+
"""If a more than one mapping exists of the same name, none of them can
856+
be resized: they'll raise an Exception and leave the original mapping intact
857+
"""
858+
start_size = 2 * PAGESIZE
859+
reduced_size = PAGESIZE
860+
tagname = "TEST"
861+
data_length = 8
862+
data = bytes(random.getrandbits(8) for _ in range(data_length))
863+
864+
m1 = mmap.mmap(-1, start_size, tagname=tagname)
865+
m2 = mmap.mmap(-1, start_size, tagname=tagname)
866+
m1[:data_length] = data
867+
self.assertEqual(m2[:data_length], data)
868+
with self.assertRaises(OSError):
869+
m1.resize(reduced_size)
870+
self.assertEqual(m1.size(), start_size)
871+
self.assertEqual(m1[:data_length], data)
872+
self.assertEqual(m2[:data_length], data)
799873

800874
class LargeMmapTests(unittest.TestCase):
801875

Modules/mmapmodule.c

Lines changed: 95 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
#ifdef MS_WINDOWS
3434
#include <windows.h>
35+
#include <winternl.h>
3536
static int
3637
my_getpagesize(void)
3738
{
@@ -376,14 +377,15 @@ is_resizeable(mmap_object *self)
376377
{
377378
if (self->exports > 0) {
378379
PyErr_SetString(PyExc_BufferError,
379-
"mmap can't resize with extant buffers exported.");
380+
"mmap can't resize with extant buffers exported.");
380381
return 0;
381382
}
382383
if ((self->access == ACCESS_WRITE) || (self->access == ACCESS_DEFAULT))
383384
return 1;
384385
PyErr_Format(PyExc_TypeError,
385-
"mmap can't resize a readonly or copy-on-write memory map.");
386+
"mmap can't resize a readonly or copy-on-write memory map.");
386387
return 0;
388+
387389
}
388390

389391

@@ -503,51 +505,110 @@ mmap_resize_method(mmap_object *self,
503505
}
504506

505507
{
508+
/*
509+
To resize an mmap on Windows:
510+
511+
- Close the existing mapping
512+
- If the mapping is backed to a named file:
513+
unmap the view, clear the data, and resize the file
514+
If the file can't be resized (eg because it has other mapped references
515+
to it) then let the mapping be recreated at the original size and set
516+
an error code so an exception will be raised.
517+
- Create a new mapping of the relevant size to the same file
518+
- Map a new view of the resized file
519+
- If the mapping is backed by the pagefile:
520+
copy any previous data into the new mapped area
521+
unmap the original view which will release the memory
522+
*/
506523
#ifdef MS_WINDOWS
507-
DWORD dwErrCode = 0;
508-
DWORD off_hi, off_lo, newSizeLow, newSizeHigh;
509-
/* First, unmap the file view */
510-
UnmapViewOfFile(self->data);
511-
self->data = NULL;
512-
/* Close the mapping object */
524+
DWORD error = 0, file_resize_error = 0;
525+
char* old_data = self->data;
526+
LARGE_INTEGER offset, max_size;
527+
offset.QuadPart = self->offset;
528+
max_size.QuadPart = self->offset + new_size;
529+
/* close the file mapping */
513530
CloseHandle(self->map_handle);
514-
self->map_handle = NULL;
515-
/* Move to the desired EOF position */
516-
newSizeHigh = (DWORD)((self->offset + new_size) >> 32);
517-
newSizeLow = (DWORD)((self->offset + new_size) & 0xFFFFFFFF);
518-
off_hi = (DWORD)(self->offset >> 32);
519-
off_lo = (DWORD)(self->offset & 0xFFFFFFFF);
520-
SetFilePointer(self->file_handle,
521-
newSizeLow, &newSizeHigh, FILE_BEGIN);
522-
/* Change the size of the file */
523-
SetEndOfFile(self->file_handle);
524-
/* Create another mapping object and remap the file view */
531+
/* if the file mapping still exists, it cannot be resized. */
532+
if (self->tagname) {
533+
self->map_handle = OpenFileMapping(FILE_MAP_WRITE, FALSE,
534+
self->tagname);
535+
if (self->map_handle) {
536+
PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE);
537+
return NULL;
538+
}
539+
} else {
540+
self->map_handle = NULL;
541+
}
542+
543+
/* if it's not the paging file, unmap the view and resize the file */
544+
if (self->file_handle != INVALID_HANDLE_VALUE) {
545+
if (!UnmapViewOfFile(self->data)) {
546+
return PyErr_SetFromWindowsErr(GetLastError());
547+
};
548+
self->data = NULL;
549+
/* resize the file */
550+
if (!SetFilePointerEx(self->file_handle, max_size, NULL,
551+
FILE_BEGIN) ||
552+
!SetEndOfFile(self->file_handle)) {
553+
/* resizing failed. try to remap the file */
554+
file_resize_error = GetLastError();
555+
new_size = max_size.QuadPart = self->size;
556+
}
557+
}
558+
559+
/* create a new file mapping and map a new view */
560+
/* FIXME: call CreateFileMappingW with wchar_t tagname */
525561
self->map_handle = CreateFileMapping(
526562
self->file_handle,
527563
NULL,
528564
PAGE_READWRITE,
529-
0,
530-
0,
565+
max_size.HighPart,
566+
max_size.LowPart,
531567
self->tagname);
532-
if (self->map_handle != NULL) {
533-
self->data = (char *) MapViewOfFile(self->map_handle,
534-
FILE_MAP_WRITE,
535-
off_hi,
536-
off_lo,
537-
new_size);
568+
569+
error = GetLastError();
570+
if (error == ERROR_ALREADY_EXISTS) {
571+
CloseHandle(self->map_handle);
572+
self->map_handle = NULL;
573+
}
574+
else if (self->map_handle != NULL) {
575+
self->data = MapViewOfFile(self->map_handle,
576+
FILE_MAP_WRITE,
577+
offset.HighPart,
578+
offset.LowPart,
579+
new_size);
538580
if (self->data != NULL) {
581+
/* copy the old view if using the paging file */
582+
if (self->file_handle == INVALID_HANDLE_VALUE) {
583+
memcpy(self->data, old_data,
584+
self->size < new_size ? self->size : new_size);
585+
if (!UnmapViewOfFile(old_data)) {
586+
error = GetLastError();
587+
}
588+
}
539589
self->size = new_size;
540-
Py_RETURN_NONE;
541-
} else {
542-
dwErrCode = GetLastError();
590+
}
591+
else {
592+
error = GetLastError();
543593
CloseHandle(self->map_handle);
544594
self->map_handle = NULL;
545595
}
546-
} else {
547-
dwErrCode = GetLastError();
548596
}
549-
PyErr_SetFromWindowsErr(dwErrCode);
550-
return NULL;
597+
598+
if (error) {
599+
return PyErr_SetFromWindowsErr(error);
600+
return NULL;
601+
}
602+
/* It's possible for a resize to fail, typically because another mapping
603+
is still held against the same underlying file. Even if nothing has
604+
failed -- ie we're still returning a valid file mapping -- raise the
605+
error as an exception as the resize won't have happened
606+
*/
607+
if (file_resize_error) {
608+
PyErr_SetFromWindowsErr(file_resize_error);
609+
return NULL;
610+
}
611+
Py_RETURN_NONE;
551612
#endif /* MS_WINDOWS */
552613

553614
#ifdef UNIX

0 commit comments

Comments
 (0)