Skip to content

Commit e33f341

Browse files
committed
Do not unlink pre-existing POSIX shared memory on error
`multiprocessing.shared_memory.SharedMemory` currently attempts to `unlink` a POSIX shared memory object it has opened or created if a subsequent exception is raised in its constructor. This is necessary to avoid it leaking if the shared memory object was created in the constructor, however it seems inappropriate if it is opening a pre-existing shared memory object. Fix this by only unlinking the shared memory object if it was created.
1 parent 19a6526 commit e33f341

File tree

2 files changed

+24
-4
lines changed

2 files changed

+24
-4
lines changed

Lib/multiprocessing/shared_memory.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ def __init__(self, name=None, create=False, size=0, *, track=True):
116116
size = stats.st_size
117117
self._mmap = mmap.mmap(self._fd, size)
118118
except:
119-
_posixshmem.shm_unlink(self._name)
119+
if create:
120+
_posixshmem.shm_unlink(self._name)
120121
raise
121122
if self._track:
122123
resource_tracker.register(self._name, "shared_memory")

Lib/test/_test_multiprocessing.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4851,6 +4851,26 @@ def test_shared_memory_tracking(self):
48514851
resource_tracker.unregister(mem._name, "shared_memory")
48524852
mem.close()
48534853

4854+
@unittest.skipIf(os.name != "posix", "windows automatically unlinks")
4855+
def test_creating_shm_unlinks_on_error(self):
4856+
name = self._new_shm_name("test_creating_shm_unlinks_on_error")
4857+
with unittest.mock.patch('mmap.mmap') as mock_mmap:
4858+
mock_mmap.side_effect = OSError("filesystems are evil")
4859+
with self.assertRaises(OSError):
4860+
shared_memory.SharedMemory(name, create=True, size=1)
4861+
with self.assertRaises(FileNotFoundError):
4862+
import _posixshmem
4863+
_posixshmem.shm_unlink(name)
4864+
4865+
def test_existing_shm_not_unlinked_on_error(self):
4866+
name = self._new_shm_name("test_existing_shm_not_unlinked_on_error")
4867+
mem = shared_memory.SharedMemory(name, create=True, size=1)
4868+
self.addCleanup(mem.unlink)
4869+
with unittest.mock.patch('mmap.mmap') as mock_mmap:
4870+
mock_mmap.side_effect = OSError("filesystems are evil")
4871+
with self.assertRaises(OSError):
4872+
shared_memory.SharedMemory(name, create=False)
4873+
48544874
@unittest.skipIf(os.name != "posix", "posix-only test w/ empty file")
48554875
def test_cleanup_zero_length_shared_memory(self):
48564876
import _posixshmem
@@ -4864,9 +4884,8 @@ def test_cleanup_zero_length_shared_memory(self):
48644884
with self.assertRaises(ValueError):
48654885
shared_memory.SharedMemory(name, create=False)
48664886

4867-
# ...however it should delete the shared memory as part of its cleanup
4868-
with self.assertRaises(FileNotFoundError):
4869-
_posixshmem.shm_open(name, os.O_RDWR)
4887+
# ...but it should NOT delete the shared memory as part of its cleanup
4888+
_posixshmem.shm_unlink(name)
48704889

48714890
#
48724891
# Test to verify that `Finalize` works.

0 commit comments

Comments
 (0)