Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[multiprocessing] memory leak in shared_memory.SharedMemory in Windows #85059

Closed
eryksun opened this issue Jun 5, 2020 · 2 comments
Closed
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes OS-windows stdlib Python modules in the Lib dir topic-multiprocessing type-bug An unexpected behavior, bug, or error

Comments

@eryksun
Copy link
Contributor

eryksun commented Jun 5, 2020

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
    Copy link
    Contributor Author

    eryksun commented Jun 5, 2020

    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 eryksun added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir OS-windows type-bug An unexpected behavior, bug, or error labels Jun 5, 2020
    @eryksun
    Copy link
    Contributor Author

    eryksun commented Jun 8, 2020

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @eryksun eryksun changed the title memory leak in multiprocessing.shared_memory.SharedMemory in Windows [multiprocessing] memory leak in shared_memory.SharedMemory in Windows Aug 8, 2022
    @eryksun eryksun closed this as completed Dec 2, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes OS-windows stdlib Python modules in the Lib dir topic-multiprocessing type-bug An unexpected behavior, bug, or error
    Projects
    Development

    No branches or pull requests

    2 participants