Skip to content

Commit 759ae67

Browse files
committed
gh-153279: Don't unlink another process's shared memory block on failed attach
SharedMemory.__init__ called self.unlink() from its OSError cleanup handler unconditionally. On the attach path (create=False) this destroyed a block owned by another process when mmap() failed (e.g. ENOMEM). The cleanup now only closes the file descriptor and unlinks the block only when it was created in the failing call. Because the block has not been registered with the resource tracker yet at that point, it is unlinked directly to avoid a spurious UNREGISTER (which raised KeyError in the resource tracker on the create path).
1 parent 4f39efc commit 759ae67

3 files changed

Lines changed: 52 additions & 1 deletion

File tree

Lib/multiprocessing/shared_memory.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,14 @@ 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 OSError:
119-
self.unlink()
119+
self.close()
120+
# Only destroy the shared memory block if we created it in
121+
# this call. When attaching to a block created by another
122+
# process we must not unlink another owner's memory. The
123+
# block has not been registered with the resource tracker
124+
# yet, so unlink it directly to avoid a spurious UNREGISTER.
125+
if create:
126+
_posixshmem.shm_unlink(self._name)
120127
raise
121128
if self._track:
122129
resource_tracker.register(self._name, "shared_memory")

Lib/test/_test_multiprocessing.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4615,6 +4615,44 @@ class OptionalAttachSharedMemory(shared_memory.SharedMemory):
46154615

46164616
sms.close()
46174617

4618+
@unittest.skipUnless(shared_memory._USE_POSIX,
4619+
"POSIX shared memory required")
4620+
def test_shared_memory_attach_failure_preserves_block(self):
4621+
# A failure to mmap while *attaching* to an existing shared memory
4622+
# block (create=False) must not unlink (destroy) the block, which
4623+
# is owned by another party.
4624+
name = self._new_shm_name('test_attach_fail')
4625+
owner = shared_memory.SharedMemory(name, create=True, size=1024)
4626+
self.addCleanup(owner.unlink)
4627+
self.addCleanup(owner.close)
4628+
4629+
with unittest.mock.patch(
4630+
'multiprocessing.shared_memory.mmap.mmap',
4631+
side_effect=OSError("simulated mmap failure")):
4632+
with self.assertRaises(OSError):
4633+
shared_memory.SharedMemory(name) # attach must not unlink
4634+
4635+
# The owner's block must still be attachable.
4636+
attached = shared_memory.SharedMemory(name)
4637+
self.addCleanup(attached.close)
4638+
self.assertGreaterEqual(attached.size, 1024)
4639+
4640+
@unittest.skipUnless(shared_memory._USE_POSIX,
4641+
"POSIX shared memory required")
4642+
def test_shared_memory_create_failure_cleans_up(self):
4643+
# A failure to mmap while *creating* a block must destroy the
4644+
# just-created block so that it is not leaked.
4645+
name = self._new_shm_name('test_create_fail')
4646+
with unittest.mock.patch(
4647+
'multiprocessing.shared_memory.mmap.mmap',
4648+
side_effect=OSError("simulated mmap failure")):
4649+
with self.assertRaises(OSError):
4650+
shared_memory.SharedMemory(name, create=True, size=1024)
4651+
4652+
# The half-created block must not survive.
4653+
with self.assertRaises(FileNotFoundError):
4654+
shared_memory.SharedMemory(name)
4655+
46184656
def test_shared_memory_recreate(self):
46194657
# Test if shared memory segment is created properly,
46204658
# when _make_filename returns an existing shared memory segment name
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Fix :class:`multiprocessing.shared_memory.SharedMemory` destroying another
2+
process's shared memory block when attaching to it fails. A failure to
3+
:func:`~mmap.mmap` while attaching (``create=False``) no longer unlinks the
4+
block, which is owned by another process; the file descriptor is simply
5+
closed. The block is now unlinked only when it was created in the failing
6+
call.

0 commit comments

Comments
 (0)