From d8656e6f85071f70fdd02f8dd3ba1980e4a9ae41 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Sat, 23 Oct 2021 21:28:39 +0100 Subject: [PATCH 01/10] On Windows, if a named mmap section is held by more than one handle, no handle can be resized --- Lib/test/test_mmap.py | 20 ++++++++++++++++++++ Modules/mmapmodule.c | 26 ++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 8f34c182f82eaf..49c91364f611b4 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -796,6 +796,26 @@ 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_fails_if_named_section_held_elsewhere(self): + """If a named section is mapped more than once on Windows, neither + mapping can be resized + """ + start_size = 2 * PAGESIZE + reduced_size = PAGESIZE + tagname = "TEST" + + f = open(TESTFN, 'wb+') + try: + m1 = mmap.mmap(f.fileno(), start_size, tagname) + m2 = mmap.mmap(f.fileno(), start_size, tagname) + with self.assertRaises(OSError): + m1.resize(reduced_size) + m2.close() + m1.resize(reduced_size) + self.assertEqual(len(m1), reduced_size) + finally: + f.close() class LargeMmapTests(unittest.TestCase): diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 6397b0d4b81095..543785e01f6d8b 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -32,6 +32,7 @@ #ifdef MS_WINDOWS #include +#include static int my_getpagesize(void) { @@ -374,16 +375,37 @@ is_writable(mmap_object *self) static int is_resizeable(mmap_object *self) { +#ifdef MS_WINDOWS + /* a named file mapping that's open more than once can't be resized */ + /* this check could be moved into is_resizeable */ + if (self->tagname) { + typedef NTSTATUS NTAPI ntqo_f(HANDLE, OBJECT_INFORMATION_CLASS, + PVOID, ULONG, PULONG); + ntqo_f *pNtQueryObject = (ntqo_f *)GetProcAddress( + GetModuleHandleW(L"ntdll"), "NtQueryObject"); + if (pNtQueryObject) { + PUBLIC_OBJECT_BASIC_INFORMATION info; + NTSTATUS status = pNtQueryObject(self->map_handle, + ObjectBasicInformation, &info, sizeof(info), NULL); + if (NT_SUCCESS(status) && info.HandleCount > 1) { + PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE); + return 0; + } + } + } +#endif (MS_WINDOWS) + 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; + } From f9750eb60c02f918e8b4cdaac2de31a5f58bde51 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Sun, 24 Oct 2021 10:13:47 +0100 Subject: [PATCH 02/10] A pagefile-backed mmap can now be resized (under the cover it's being recreated along with its data) --- Lib/test/test_mmap.py | 17 +++++++++ Modules/mmapmodule.c | 85 +++++++++++++++++++++++++++---------------- 2 files changed, 71 insertions(+), 31 deletions(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 49c91364f611b4..6e20d273290a40 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 @@ -817,6 +818,22 @@ def test_resize_fails_if_named_section_held_elsewhere(self): finally: f.close() + @unittest.skipUnless(os.name == 'nt', 'requires Windows') + def test_resize_when_mapped_to_pagefile(self): + """If the mmap is backed by the pagefile ensure a resize can happen + """ + start_size = 6 + increased_size = 2 * start_size + data = bytes(random.getrandbits(8) for _ in range(start_size)) + + m = mmap.mmap(-1, start_size) + m[:] = data + m.resize(increased_size) + self.assertEqual(len(m), increased_size) + self.assertEqual(m[:start_size], data) + + + class LargeMmapTests(unittest.TestCase): def setUp(self): diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 543785e01f6d8b..64ec01fa50f9d8 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -526,50 +526,73 @@ mmap_resize_method(mmap_object *self, { #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; + char* old_data = self->data; + LARGE_INTEGER offset, max_size; + offset.QuadPart = self->offset; + max_size.QuadPart = 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 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 */ + 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); + if (self->map_handle != NULL && + GetLastError() != ERROR_ALREADY_EXISTS) { + 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 + ); + } 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; + else { + error = GetLastError(); + } + /* unmap the old view if using the paging file */ + if (self->file_handle == INVALID_HANDLE_VALUE) { + UnmapViewOfFile(old_data); + } + if (error) { + PyErr_SetFromWindowsErr(error); + return NULL; + } + Py_RETURN_NONE; #endif /* MS_WINDOWS */ #ifdef UNIX From b114bc14525f5f7d7af52957341a34bc55c6fa08 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Sun, 24 Oct 2021 23:02:29 +0100 Subject: [PATCH 03/10] Make sure that resizing includes the offset when calculating the new file extent --- Modules/mmapmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 64ec01fa50f9d8..f1f70406327744 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -530,7 +530,7 @@ mmap_resize_method(mmap_object *self, char* old_data = self->data; LARGE_INTEGER offset, max_size; offset.QuadPart = self->offset; - max_size.QuadPart = new_size; + max_size.QuadPart = self->offset + new_size; /* close the file mapping */ CloseHandle(self->map_handle); self->map_handle = NULL; From 2742cad828e075829de61fcde795cfd8dcafbef4 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Mon, 25 Oct 2021 08:20:01 +0100 Subject: [PATCH 04/10] Add & tweak resize tests for Windows: * Check that a resize against the pagefile succeeds and copies as much data as will fit * Check that a resize against a named file correctly fails where another mapping is still open --- Lib/test/test_mmap.py | 60 ++++++++++++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 6e20d273290a40..b1bc39439b092b 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -798,8 +798,38 @@ def test_madvise(self): self.assertEqual(m.madvise(mmap.MADV_NORMAL, 0, size), None) @unittest.skipUnless(os.name == 'nt', 'requires Windows') - def test_resize_fails_if_named_section_held_elsewhere(self): - """If a named section is mapped more than once on Windows, neither + 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 @@ -807,33 +837,21 @@ def test_resize_fails_if_named_section_held_elsewhere(self): tagname = "TEST" f = open(TESTFN, 'wb+') + f.truncate(start_size) try: - m1 = mmap.mmap(f.fileno(), start_size, tagname) - m2 = mmap.mmap(f.fileno(), start_size, tagname) + 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(len(m1), 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_when_mapped_to_pagefile(self): - """If the mmap is backed by the pagefile ensure a resize can happen - """ - start_size = 6 - increased_size = 2 * start_size - data = bytes(random.getrandbits(8) for _ in range(start_size)) - - m = mmap.mmap(-1, start_size) - m[:] = data - m.resize(increased_size) - self.assertEqual(len(m), increased_size) - self.assertEqual(m[:start_size], data) - - - class LargeMmapTests(unittest.TestCase): def setUp(self): From 00f3be6f40aaa667758d93a2be25343e7271e1c0 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Mon, 25 Oct 2021 08:28:00 +0100 Subject: [PATCH 05/10] Add useful commentary to the new resize logic --- Modules/mmapmodule.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index f1f70406327744..a6c1269417166f 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -377,7 +377,6 @@ is_resizeable(mmap_object *self) { #ifdef MS_WINDOWS /* a named file mapping that's open more than once can't be resized */ - /* this check could be moved into is_resizeable */ if (self->tagname) { typedef NTSTATUS NTAPI ntqo_f(HANDLE, OBJECT_INFORMATION_CLASS, PVOID, ULONG, PULONG); @@ -525,6 +524,21 @@ 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 error = 0; char* old_data = self->data; @@ -586,7 +600,9 @@ mmap_resize_method(mmap_object *self, } /* unmap the old view if using the paging file */ if (self->file_handle == INVALID_HANDLE_VALUE) { - UnmapViewOfFile(old_data); + if(!UnmapViewOfFile(old_data)) { + error = GetLastError(); + }; } if (error) { PyErr_SetFromWindowsErr(error); From ea480100265e2a1c1744bc17fed0e373d81e828e Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Mon, 25 Oct 2021 16:23:51 +0100 Subject: [PATCH 06/10] Add some tentative docs to reflect the new changes --- Doc/library/mmap.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Doc/library/mmap.rst b/Doc/library/mmap.rst index da174753361b2f..e7a9fe67bcf189 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 fail 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]]) From b1a6c5f2c7df73702c210692c30c66f3f6962f30 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Mon, 25 Oct 2021 20:35:15 +0100 Subject: [PATCH 07/10] Add a specific test to check that resizing a named mapping fails with an exception but retains the mapping Remove the specific pre-check on resizing a named section but specifically account for it later when resizing --- Lib/test/test_mmap.py | 23 +++++++++++++++++++++-- Modules/mmapmodule.c | 28 +++++++--------------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index b1bc39439b092b..82e2d2adb7dcfc 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -708,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) @@ -834,7 +833,6 @@ def test_resize_fails_if_mapping_held_elsewhere(self): """ start_size = 2 * PAGESIZE reduced_size = PAGESIZE - tagname = "TEST" f = open(TESTFN, 'wb+') f.truncate(start_size) @@ -852,6 +850,27 @@ def test_resize_fails_if_mapping_held_elsewhere(self): 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): def setUp(self): diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index a6c1269417166f..77408a43a14fb1 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -375,25 +375,6 @@ is_writable(mmap_object *self) static int is_resizeable(mmap_object *self) { -#ifdef MS_WINDOWS - /* a named file mapping that's open more than once can't be resized */ - if (self->tagname) { - typedef NTSTATUS NTAPI ntqo_f(HANDLE, OBJECT_INFORMATION_CLASS, - PVOID, ULONG, PULONG); - ntqo_f *pNtQueryObject = (ntqo_f *)GetProcAddress( - GetModuleHandleW(L"ntdll"), "NtQueryObject"); - if (pNtQueryObject) { - PUBLIC_OBJECT_BASIC_INFORMATION info; - NTSTATUS status = pNtQueryObject(self->map_handle, - ObjectBasicInformation, &info, sizeof(info), NULL); - if (NT_SUCCESS(status) && info.HandleCount > 1) { - PyErr_SetFromWindowsErr(ERROR_USER_MAPPED_FILE); - return 0; - } - } - } -#endif (MS_WINDOWS) - if (self->exports > 0) { PyErr_SetString(PyExc_BufferError, "mmap can't resize with extant buffers exported."); @@ -598,8 +579,13 @@ mmap_resize_method(mmap_object *self, else { error = GetLastError(); } - /* unmap the old view if using the paging file */ - if (self->file_handle == INVALID_HANDLE_VALUE) { + /* unmap the old view if using the paging file + Don't do this if we've encountered ERROR_ALREADY_EXISTS as this means + that creating a new named file mapping failed and the mapping handle + points to an existing mapping of the same name + */ + if ((GetLastError() != ERROR_ALREADY_EXISTS) && + (self->file_handle == INVALID_HANDLE_VALUE)) { if(!UnmapViewOfFile(old_data)) { error = GetLastError(); }; From 866a376a4a340dffe9b2d4c2e98bc1b1e8af2f47 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Tue, 26 Oct 2021 04:54:10 +0100 Subject: [PATCH 08/10] Use the existing `error` value as `GetLastError` might return a different value by this point --- Modules/mmapmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 77408a43a14fb1..d4d090cc6aa6a9 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -584,7 +584,7 @@ mmap_resize_method(mmap_object *self, that creating a new named file mapping failed and the mapping handle points to an existing mapping of the same name */ - if ((GetLastError() != ERROR_ALREADY_EXISTS) && + if ((error != ERROR_ALREADY_EXISTS) && (self->file_handle == INVALID_HANDLE_VALUE)) { if(!UnmapViewOfFile(old_data)) { error = GetLastError(); From eef2b796c0055bb0dd91ffd986a69eb324e52a09 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Tue, 26 Oct 2021 08:54:28 +0100 Subject: [PATCH 09/10] Be specific about the exception type raised --- Doc/library/mmap.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Doc/library/mmap.rst b/Doc/library/mmap.rst index e7a9fe67bcf189..c1ebd80abd977b 100644 --- a/Doc/library/mmap.rst +++ b/Doc/library/mmap.rst @@ -256,10 +256,10 @@ 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 fail 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. + **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 From af636d62d47a9d474e34c735a1b7d95d89cdb688 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Tue, 26 Oct 2021 18:05:00 +0100 Subject: [PATCH 10/10] Pick up an error when resizing the file even if everything else succeeds --- Modules/mmapmodule.c | 60 +++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index d4d090cc6aa6a9..dfa10f633bbd53 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -521,14 +521,25 @@ mmap_resize_method(mmap_object *self, unmap the original view which will release the memory */ #ifdef MS_WINDOWS - DWORD error = 0; + 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; + /* 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)) { @@ -540,10 +551,11 @@ mmap_resize_method(mmap_object *self, FILE_BEGIN) || !SetEndOfFile(self->file_handle)) { /* resizing failed. try to remap the file */ - error = GetLastError(); + 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( @@ -553,8 +565,13 @@ mmap_resize_method(mmap_object *self, max_size.HighPart, max_size.LowPart, self->tagname); - if (self->map_handle != NULL && - GetLastError() != ERROR_ALREADY_EXISTS) { + + 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, @@ -563,10 +580,11 @@ mmap_resize_method(mmap_object *self, 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 - ); + memcpy(self->data, old_data, + self->size < new_size ? self->size : new_size); + if (!UnmapViewOfFile(old_data)) { + error = GetLastError(); + } } self->size = new_size; } @@ -576,22 +594,18 @@ mmap_resize_method(mmap_object *self, self->map_handle = NULL; } } - else { - error = GetLastError(); + + if (error) { + return PyErr_SetFromWindowsErr(error); + return NULL; } - /* unmap the old view if using the paging file - Don't do this if we've encountered ERROR_ALREADY_EXISTS as this means - that creating a new named file mapping failed and the mapping handle - points to an existing mapping of the same name + /* 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 ((error != ERROR_ALREADY_EXISTS) && - (self->file_handle == INVALID_HANDLE_VALUE)) { - if(!UnmapViewOfFile(old_data)) { - error = GetLastError(); - }; - } - if (error) { - PyErr_SetFromWindowsErr(error); + if (file_resize_error) { + PyErr_SetFromWindowsErr(file_resize_error); return NULL; } Py_RETURN_NONE;