[multiprocessing] memory leak in shared_memory.SharedMemory in Windows

[eryksun]

BPO	40882
Nosy	@pfmoore, @tiran, @tjguk, @zware, @eryksun, @zooba, @ZackerySpytz
PRs	bpo-40882: Fix a memory leak in SharedMemory on Windows #20684

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-06-05.22:47:05.789>
labels = ['type-bug', '3.8', '3.9', '3.10', 'library', 'OS-windows']
title = 'memory leak in multiprocessing.shared_memory.SharedMemory in Windows'
updated_at = <Date 2020-06-10.00:55:43.712>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2020-06-10.00:55:43.712>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2020-06-05.22:47:05.789>
creator = 'eryksun'
dependencies = []
files = []
hgrepos = []
issue_num = 40882
keywords = ['patch']
message_count = 2.0
messages = ['370794', '370955']
nosy_count = 7.0
nosy_names = ['paul.moore', 'christian.heimes', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'ZackerySpytz']
pr_nums = ['20684']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue40882'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[eryksun]

mmap.mmap in Windows doesn't support an exist_ok parameter and doesn't correctly handle the combination fileno=-1, length=0, and tagname with an existing file mapping. SharedMemory has to work around these limitations.

Part of the workaround for the create=False case requires mapping a view via MapViewOfFile in order to get the size from VirtualQuerySize, since mmap.mmap requires it (needlessly if implemented right) when fileno=-1. This mapped view never gets unmapped, which means the shared memory will never be freed until the termination of all processes that have opened it with create=False. Also, at least in a 32-bit process, this wastes precious address space.

UnmapViewOfFile needs to be implemented in the _winapi module. Then the temporary view can be unmapped as follows:

    self._name = name
    h_map = _winapi.OpenFileMapping(_winapi.FILE_MAP_READ, False, name)
    try:
        p_buf = _winapi.MapViewOfFile(h_map, _winapi.FILE_MAP_READ, 0, 0, 0)
    finally:
        _winapi.CloseHandle(h_map)
    try:
        size = _winapi.VirtualQuerySize(p_buf)
    finally:
        _winapi.UnmapViewOfFile(p_buf)
    self._mmap = mmap.mmap(-1, size, tagname=name)

[eryksun]

Thanks for working on the PR, Zackery. Would you be interested in working on improvements to mmap for 3.10? With support in mmap, the Windows-specific initialization of SharedMemory could be as simple as the following:

# Windows Named Shared Memory

    while True:
        tagname = _make_filename() if name is None else name
        try:
            self._mmap = mmap.mmap(-1, size if create else 0,
                tagname, create=create)
            break
        except FileExistsError:
            if name is not None:
                raise

    self._name = tagname
    self._size = len(self._mmap)

The new mmap create parameter would default to None, which uses the current behavior that allows either opening or creating a file mapping with no sanity checking (e.g. a valid fd gets passed in, but it opens an unrelated file mapping via tagname). If create is true, and there's an existing file mapping named tagname, then raise FileExistsError. If create is false, call OpenFileMappingW instead of CreateFileMappingW. In this case, fileno must be -1, length is allowed to be 0, and tagname must be a non-empty string. If length is 0, map the entire file mapping and get the size via VirtualQuery: RegionSize.