From d4756a7350d785848a7c06733d355a1cc9156876 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 11 Sep 2021 22:09:01 +0300 Subject: [PATCH 1/4] bpo-45125: Improves pickling docs and tests for `shared_memory` --- Doc/library/multiprocessing.shared_memory.rst | 27 ++++++ Lib/test/_test_multiprocessing.py | 89 ++++++++++++++++--- .../2021-09-11-22-08-18.bpo-45125.FVSzs2.rst | 2 + 3 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2021-09-11-22-08-18.bpo-45125.FVSzs2.rst diff --git a/Doc/library/multiprocessing.shared_memory.rst b/Doc/library/multiprocessing.shared_memory.rst index cba576a29e2e2b..2ba42b7e579a77 100644 --- a/Doc/library/multiprocessing.shared_memory.rst +++ b/Doc/library/multiprocessing.shared_memory.rst @@ -342,3 +342,30 @@ behind it: >>> c.shm.close() >>> c.shm.unlink() +The following examples demonstrates that ``ShareableList`` +(and underlying ``SharedMemory``) objects +can be pickled and unpickled if needed. +Note, that it will still be the same shared object. +This happens, because the deserialized object has +the same unique name and is just attached to an existing +object with the same name (if the object is still alive): + + >>> import pickle + >>> from multiprocessing import shared_memory + >>> sl = shared_memory.ShareableList(range(10)) + >>> list(sl) + [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + + >>> deserialized_sl = pickle.loads(pickle.dumps(sl)) + >>> list(deserialized_sl) + [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] + + >>> sl[0] = -1 + >>> deserialized_sl[1] = -2 + >>> list(sl) + [-1, -2, 2, 3, 4, 5, 6, 7, 8, 9] + >>> list(deserialized_sl) + [-1, -2, 2, 3, 4, 5, 6, 7, 8, 9] + + >>> sl.shm.close() + >>> sl.shm.unlink() diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 9e0d18da50f6ef..ea4d3f08ccbdd7 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3793,13 +3793,6 @@ def test_shared_memory_basics(self): self.assertIn(sms.name, str(sms)) self.assertIn(str(sms.size), str(sms)) - # Test pickling - sms.buf[0:6] = b'pickle' - pickled_sms = pickle.dumps(sms) - sms2 = pickle.loads(pickled_sms) - self.assertEqual(sms.name, sms2.name) - self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'pickle') - # Modify contents of shared memory segment through memoryview. sms.buf[0] = 42 self.assertEqual(sms.buf[0], 42) @@ -3898,6 +3891,29 @@ class OptionalAttachSharedMemory(shared_memory.SharedMemory): sms.close() + def test_shared_memory_recreate(self): + # Test if shared memory segment is created properly, + # when _make_filename returns an existing shared memory segment name + with unittest.mock.patch( + 'multiprocessing.shared_memory._make_filename') as mock_make_filename: + + NAME_PREFIX = shared_memory._SHM_NAME_PREFIX + names = ['test01_fn', 'test02_fn'] + # Prepend NAME_PREFIX which can be '/psm_' or 'wnsm_', necessary + # because some POSIX compliant systems require name to start with / + names = [NAME_PREFIX + name for name in names] + + mock_make_filename.side_effect = names + shm1 = shared_memory.SharedMemory(create=True, size=1) + self.addCleanup(shm1.unlink) + self.assertEqual(shm1._name, names[0]) + + mock_make_filename.side_effect = names + shm2 = shared_memory.SharedMemory(create=True, size=1) + self.addCleanup(shm2.unlink) + self.assertEqual(shm2._name, names[1]) + + def test_invalid_shared_memory_cration(self): # Test creating a shared memory segment with negative size with self.assertRaises(ValueError): sms_invalid = shared_memory.SharedMemory(create=True, size=-1) @@ -3910,6 +3926,41 @@ class OptionalAttachSharedMemory(shared_memory.SharedMemory): with self.assertRaises(ValueError): sms_invalid = shared_memory.SharedMemory(create=True) + def test_shared_memory_pickle_unpickle(self): + sms = shared_memory.SharedMemory(create=True, size=512) + self.addCleanup(sms.unlink) + sms.buf[0:6] = b'pickle' + + # Test pickling + pickled_sms = pickle.dumps(sms) + self.assertNotIn(b'pickle', pickled_sms) + + # Test unpickling + sms2 = pickle.loads(pickled_sms) + self.assertIsInstance(sms2, sms.__class__) + self.assertEqual(sms.name, sms2.name) + self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'pickle') + + # Test that unpickled version is still the same SharedMemory + sms.buf[0:6] = b'newval' + self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'newval') + + sms2.buf[0:6] = b'oldval' + self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'oldval') + + def test_shared_memory_pickle_unpickle_dead_object(self): + sms = shared_memory.SharedMemory(create=True, size=512) + sms.buf[0:6] = b'pickle' + pickled_sms = pickle.dumps(sms) + + # Now, we are going to kill the original object. + # So, unpickled one won't be able to attach to it. + sms.close() + sms.unlink() + + with self.assertRaises(FileNotFoundError): + pickle.loads(pickled_sms) + def test_shared_memory_across_processes(self): # bpo-40135: don't define shared memory block's name in case of # the failure when we run multiprocessing tests in parallel. @@ -4132,11 +4183,9 @@ def test_shared_memory_ShareableList_pickling(self): serialized_sl = pickle.dumps(sl) deserialized_sl = pickle.loads(serialized_sl) - self.assertTrue( - isinstance(deserialized_sl, shared_memory.ShareableList) - ) - self.assertTrue(deserialized_sl[-1], 9) - self.assertFalse(sl is deserialized_sl) + self.assertIsInstance(deserialized_sl, shared_memory.ShareableList) + self.assertEqual(deserialized_sl[-1], 9) + self.assertIsNot(sl, deserialized_sl) deserialized_sl[4] = "changed" self.assertEqual(sl[4], "changed") @@ -4145,12 +4194,24 @@ def test_shared_memory_ShareableList_pickling(self): larger_sl = shared_memory.ShareableList(range(400)) self.addCleanup(larger_sl.shm.unlink) serialized_larger_sl = pickle.dumps(larger_sl) - self.assertTrue(len(serialized_sl) == len(serialized_larger_sl)) + self.assertEqual(len(serialized_sl), len(serialized_larger_sl)) larger_sl.shm.close() deserialized_sl.shm.close() sl.shm.close() + def test_shared_memory_ShareableList_pickling_dead_object(self): + sl = shared_memory.ShareableList(range(10)) + serialized_sl = pickle.dumps(sl) + + # Now, we are going to kill the original object. + # So, unpickled one won't be able to attach to it. + sl.shm.close() + sl.shm.unlink() + + with self.assertRaises(FileNotFoundError): + pickle.loads(serialized_sl) + def test_shared_memory_cleaned_after_process_termination(self): cmd = '''if 1: import os, time, sys @@ -4202,7 +4263,7 @@ def test_shared_memory_cleaned_after_process_termination(self): "shared_memory objects to clean up at shutdown", err) # -# +# Test to verify that `Finalize` works. # class _TestFinalize(BaseTestCase): diff --git a/Misc/NEWS.d/next/Tests/2021-09-11-22-08-18.bpo-45125.FVSzs2.rst b/Misc/NEWS.d/next/Tests/2021-09-11-22-08-18.bpo-45125.FVSzs2.rst new file mode 100644 index 00000000000000..5dfbe0e5db4631 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2021-09-11-22-08-18.bpo-45125.FVSzs2.rst @@ -0,0 +1,2 @@ +Improves pickling tests and docs of ``SharedMemory`` and ``SharableList`` +objects. From 301df7e63a21b328268945b6aa3c357f8099dad4 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 11 Sep 2021 22:40:48 +0300 Subject: [PATCH 2/4] Adds subTest for each pickle protocol --- Lib/test/_test_multiprocessing.py | 90 +++++++++++++++++-------------- 1 file changed, 49 insertions(+), 41 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index ea4d3f08ccbdd7..65b2ad505c79aa 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3927,26 +3927,31 @@ def test_invalid_shared_memory_cration(self): sms_invalid = shared_memory.SharedMemory(create=True) def test_shared_memory_pickle_unpickle(self): - sms = shared_memory.SharedMemory(create=True, size=512) - self.addCleanup(sms.unlink) - sms.buf[0:6] = b'pickle' - - # Test pickling - pickled_sms = pickle.dumps(sms) - self.assertNotIn(b'pickle', pickled_sms) - - # Test unpickling - sms2 = pickle.loads(pickled_sms) - self.assertIsInstance(sms2, sms.__class__) - self.assertEqual(sms.name, sms2.name) - self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'pickle') - - # Test that unpickled version is still the same SharedMemory - sms.buf[0:6] = b'newval' - self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'newval') - - sms2.buf[0:6] = b'oldval' - self.assertEqual(bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'oldval') + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto): + sms = shared_memory.SharedMemory(create=True, size=512) + self.addCleanup(sms.unlink) + sms.buf[0:6] = b'pickle' + + # Test pickling + pickled_sms = pickle.dumps(sms, protocol=proto) + self.assertNotIn(b'pickle', pickled_sms) + + # Test unpickling + sms2 = pickle.loads(pickled_sms) + self.assertIsInstance(sms2, sms.__class__) + self.assertEqual(sms.name, sms2.name) + self.assertEqual( + bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'pickle') + + # Test that unpickled version is still the same SharedMemory + sms.buf[0:6] = b'newval' + self.assertEqual( + bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'newval') + + sms2.buf[0:6] = b'oldval' + self.assertEqual( + bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'oldval') def test_shared_memory_pickle_unpickle_dead_object(self): sms = shared_memory.SharedMemory(create=True, size=512) @@ -4178,27 +4183,30 @@ def test_shared_memory_ShareableList_basics(self): empty_sl.shm.unlink() def test_shared_memory_ShareableList_pickling(self): - sl = shared_memory.ShareableList(range(10)) - self.addCleanup(sl.shm.unlink) - - serialized_sl = pickle.dumps(sl) - deserialized_sl = pickle.loads(serialized_sl) - self.assertIsInstance(deserialized_sl, shared_memory.ShareableList) - self.assertEqual(deserialized_sl[-1], 9) - self.assertIsNot(sl, deserialized_sl) - deserialized_sl[4] = "changed" - self.assertEqual(sl[4], "changed") - - # Verify data is not being put into the pickled representation. - name = 'a' * len(sl.shm.name) - larger_sl = shared_memory.ShareableList(range(400)) - self.addCleanup(larger_sl.shm.unlink) - serialized_larger_sl = pickle.dumps(larger_sl) - self.assertEqual(len(serialized_sl), len(serialized_larger_sl)) - larger_sl.shm.close() - - deserialized_sl.shm.close() - sl.shm.close() + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto): + sl = shared_memory.ShareableList(range(10)) + self.addCleanup(sl.shm.unlink) + + serialized_sl = pickle.dumps(sl, protocol=proto) + deserialized_sl = pickle.loads(serialized_sl) + self.assertIsInstance( + deserialized_sl, shared_memory.ShareableList) + self.assertEqual(deserialized_sl[-1], 9) + self.assertIsNot(sl, deserialized_sl) + deserialized_sl[4] = "changed" + self.assertEqual(sl[4], "changed") + + # Verify data is not being put into the pickled representation. + name = 'a' * len(sl.shm.name) + larger_sl = shared_memory.ShareableList(range(400)) + self.addCleanup(larger_sl.shm.unlink) + serialized_larger_sl = pickle.dumps(larger_sl, protocol=proto) + self.assertEqual(len(serialized_sl), len(serialized_larger_sl)) + larger_sl.shm.close() + + deserialized_sl.shm.close() + sl.shm.close() def test_shared_memory_ShareableList_pickling_dead_object(self): sl = shared_memory.ShareableList(range(10)) From 1bb847c8b4f23d1f5ef6dd47d13a77e053a7de65 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 1 Oct 2021 11:59:19 +0300 Subject: [PATCH 3/4] Address review --- Lib/test/_test_multiprocessing.py | 58 +++++++++++++++++-------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 65b2ad505c79aa..8c25baa39838c6 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3935,36 +3935,37 @@ def test_shared_memory_pickle_unpickle(self): # Test pickling pickled_sms = pickle.dumps(sms, protocol=proto) - self.assertNotIn(b'pickle', pickled_sms) # Test unpickling sms2 = pickle.loads(pickled_sms) - self.assertIsInstance(sms2, sms.__class__) + self.assertIsInstance(sms2, shared_memory.SharedMemory) self.assertEqual(sms.name, sms2.name) - self.assertEqual( - bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'pickle') + self.assertEqual(bytes(sms.buf[0:6]), b'pickle') + self.assertEqual(bytes(sms2.buf[0:6]), b'pickle') # Test that unpickled version is still the same SharedMemory sms.buf[0:6] = b'newval' - self.assertEqual( - bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'newval') + self.assertEqual(bytes(sms.buf[0:6]), b'newval') + self.assertEqual(bytes(sms2.buf[0:6]), b'newval') sms2.buf[0:6] = b'oldval' - self.assertEqual( - bytes(sms.buf[0:6]), bytes(sms2.buf[0:6]), b'oldval') + self.assertEqual(bytes(sms.buf[0:6]), b'oldval') + self.assertEqual(bytes(sms2.buf[0:6]), b'oldval') def test_shared_memory_pickle_unpickle_dead_object(self): - sms = shared_memory.SharedMemory(create=True, size=512) - sms.buf[0:6] = b'pickle' - pickled_sms = pickle.dumps(sms) + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto): + sms = shared_memory.SharedMemory(create=True, size=512) + sms.buf[0:6] = b'pickle' + pickled_sms = pickle.dumps(sms) - # Now, we are going to kill the original object. - # So, unpickled one won't be able to attach to it. - sms.close() - sms.unlink() + # Now, we are going to kill the original object. + # So, unpickled one won't be able to attach to it. + sms.close() + sms.unlink() - with self.assertRaises(FileNotFoundError): - pickle.loads(pickled_sms) + with self.assertRaises(FileNotFoundError): + pickle.loads(pickled_sms) def test_shared_memory_across_processes(self): # bpo-40135: don't define shared memory block's name in case of @@ -4194,11 +4195,12 @@ def test_shared_memory_ShareableList_pickling(self): deserialized_sl, shared_memory.ShareableList) self.assertEqual(deserialized_sl[-1], 9) self.assertIsNot(sl, deserialized_sl) + deserialized_sl[4] = "changed" self.assertEqual(sl[4], "changed") + sl[3] = "newvalue" + self.assertEqual(deserialized_sl[3], "newvalue") - # Verify data is not being put into the pickled representation. - name = 'a' * len(sl.shm.name) larger_sl = shared_memory.ShareableList(range(400)) self.addCleanup(larger_sl.shm.unlink) serialized_larger_sl = pickle.dumps(larger_sl, protocol=proto) @@ -4209,16 +4211,18 @@ def test_shared_memory_ShareableList_pickling(self): sl.shm.close() def test_shared_memory_ShareableList_pickling_dead_object(self): - sl = shared_memory.ShareableList(range(10)) - serialized_sl = pickle.dumps(sl) + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto): + sl = shared_memory.ShareableList(range(10)) + serialized_sl = pickle.dumps(sl) - # Now, we are going to kill the original object. - # So, unpickled one won't be able to attach to it. - sl.shm.close() - sl.shm.unlink() + # Now, we are going to kill the original object. + # So, unpickled one won't be able to attach to it. + sl.shm.close() + sl.shm.unlink() - with self.assertRaises(FileNotFoundError): - pickle.loads(serialized_sl) + with self.assertRaises(FileNotFoundError): + pickle.loads(serialized_sl) def test_shared_memory_cleaned_after_process_termination(self): cmd = '''if 1: From 5de9e107ceb9678baba3583551c1f03d8c2debe0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 1 Oct 2021 12:47:46 +0300 Subject: [PATCH 4/4] Pass protocol --- Lib/test/_test_multiprocessing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 8c25baa39838c6..3bc5b8f3d79b02 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -3957,7 +3957,7 @@ def test_shared_memory_pickle_unpickle_dead_object(self): with self.subTest(proto=proto): sms = shared_memory.SharedMemory(create=True, size=512) sms.buf[0:6] = b'pickle' - pickled_sms = pickle.dumps(sms) + pickled_sms = pickle.dumps(sms, protocol=proto) # Now, we are going to kill the original object. # So, unpickled one won't be able to attach to it. @@ -4214,7 +4214,7 @@ def test_shared_memory_ShareableList_pickling_dead_object(self): for proto in range(pickle.HIGHEST_PROTOCOL + 1): with self.subTest(proto=proto): sl = shared_memory.ShareableList(range(10)) - serialized_sl = pickle.dumps(sl) + serialized_sl = pickle.dumps(sl, protocol=proto) # Now, we are going to kill the original object. # So, unpickled one won't be able to attach to it.