From 10a060f83f76997a691e497de7031a0f5cc48d08 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Wed, 27 Oct 2021 08:45:56 +0100 Subject: [PATCH 1/5] Remove unneeded include --- Modules/mmapmodule.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index dfa10f633bbd532..792082b2c719e20 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -32,7 +32,6 @@ #ifdef MS_WINDOWS #include -#include static int my_getpagesize(void) { From 45e50a02d521e5acc55a3865c9ea1b5169c2ea68 Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Wed, 27 Oct 2021 08:46:39 +0100 Subject: [PATCH 2/5] Remove verbose comment --- Modules/mmapmodule.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 792082b2c719e20..aa283485bcee5a7 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -504,21 +504,6 @@ 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, file_resize_error = 0; char* old_data = self->data; From 0e5d746c89c530fed303f48df2d5d14c43d03aec Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Wed, 27 Oct 2021 08:48:00 +0100 Subject: [PATCH 3/5] Add a comment clarifying the ERROR_ALREADY_EXISTS situation --- Modules/mmapmodule.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index aa283485bcee5a7..21d53365776ab2d 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -551,6 +551,11 @@ mmap_resize_method(mmap_object *self, self->tagname); error = GetLastError(); + /* ERROR_ALREADY_EXISTS implies that between our closing the handle above and + calling CreateFileMapping here, someone's created a different mapping with + the same name. There's nothing we can usefully do so we invalidate our + mapping and error out. + */ if (error == ERROR_ALREADY_EXISTS) { CloseHandle(self->map_handle); self->map_handle = NULL; From 8466ad55bb1343d3057b73c99326146b7d58774b Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Wed, 27 Oct 2021 08:58:21 +0100 Subject: [PATCH 4/5] Use randomized names when testing tagged mmaps to avoid any risk of parallel tests treading on each others' toes --- Lib/test/test_mmap.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 82e2d2adb7dcfc5..11704eaeb647399 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -7,6 +7,7 @@ import itertools import random import socket +import string import sys import weakref @@ -15,6 +16,8 @@ PAGESIZE = mmap.PAGESIZE +def random_tagname(length=10): + return "".join(random.choice(string.ascii_uppercase) for _ in range(length)) class MmapTests(unittest.TestCase): @@ -610,11 +613,13 @@ def test_tagname(self): data1 = b"0123456789" data2 = b"abcdefghij" assert len(data1) == len(data2) + tagname1 = random_tagname() + tagname2 = random_tagname() # Test same tag - m1 = mmap.mmap(-1, len(data1), tagname="foo") + m1 = mmap.mmap(-1, len(data1), tagname=tagname1) m1[:] = data1 - m2 = mmap.mmap(-1, len(data2), tagname="foo") + m2 = mmap.mmap(-1, len(data2), tagname=tagname1) m2[:] = data2 self.assertEqual(m1[:], data2) self.assertEqual(m2[:], data2) @@ -622,9 +627,9 @@ def test_tagname(self): m1.close() # Test different tag - m1 = mmap.mmap(-1, len(data1), tagname="foo") + m1 = mmap.mmap(-1, len(data1), tagname=tagname1) m1[:] = data1 - m2 = mmap.mmap(-1, len(data2), tagname="boo") + m2 = mmap.mmap(-1, len(data2), tagname=tagname2) m2[:] = data2 self.assertEqual(m1[:], data1) self.assertEqual(m2[:], data2) @@ -635,7 +640,7 @@ def test_tagname(self): @unittest.skipUnless(os.name == 'nt', 'requires Windows') def test_sizeof(self): m1 = mmap.mmap(-1, 100) - tagname = "foo" + tagname = random_tagname() m2 = mmap.mmap(-1, 100, tagname=tagname) self.assertEqual(sys.getsizeof(m2), sys.getsizeof(m1) + len(tagname) + 1) @@ -643,9 +648,10 @@ def test_sizeof(self): @unittest.skipUnless(os.name == 'nt', 'requires Windows') def test_crasher_on_windows(self): # Should not crash (Issue 1733986) - m = mmap.mmap(-1, 1000, tagname="foo") + tagname = random_tagname() + m = mmap.mmap(-1, 1000, tagname=tagname) try: - mmap.mmap(-1, 5000, tagname="foo")[:] # same tagname, but larger size + mmap.mmap(-1, 5000, tagname=tagname)[:] # same tagname, but larger size except: pass m.close() @@ -857,7 +863,7 @@ def test_resize_succeeds_with_error_for_second_named_mapping(self): """ start_size = 2 * PAGESIZE reduced_size = PAGESIZE - tagname = "TEST" + tagname = random_tagname() data_length = 8 data = bytes(random.getrandbits(8) for _ in range(data_length)) From c481a0a6817e26321e36a93f53a8c3458ab7cd2d Mon Sep 17 00:00:00 2001 From: Tim Golden Date: Fri, 29 Oct 2021 08:51:40 +0100 Subject: [PATCH 5/5] Tie the random mmap tag name to the Python process --- Lib/test/test_mmap.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 11704eaeb647399..014171cbb4911bc 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -16,8 +16,10 @@ PAGESIZE = mmap.PAGESIZE +tagname_prefix = f'python_{os.getpid()}_test_mmap' def random_tagname(length=10): - return "".join(random.choice(string.ascii_uppercase) for _ in range(length)) + suffix = ''.join(random.choices(string.ascii_uppercase, k=length)) + return f'{tagname_prefix}_{suffix}' class MmapTests(unittest.TestCase):