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

test_multiprocessing_spawn.WithProcessesTestPool.test_traceback() leaks 4 handles on Windows #78147

Closed
vstinner opened this issue Jun 26, 2018 · 4 comments
Labels
3.8 only security fixes OS-windows stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 33966
Nosy @pfmoore, @pitrou, @vstinner, @tjguk, @zware, @zooba, @applio
PRs
  • [WIP] bpo-33966, multiprocessing: Fix another handle leak #7965
  • bpo-33966, multiprocessing: Pool wait for worker start #7966
  • 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 = <Date 2018-09-19.23:55:24.876>
    created_at = <Date 2018-06-26.11:55:02.617>
    labels = ['3.8', 'library', 'OS-windows']
    title = 'test_multiprocessing_spawn.WithProcessesTestPool.test_traceback() leaks 4 handles on Windows'
    updated_at = <Date 2018-09-19.23:55:24.875>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2018-09-19.23:55:24.875>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-09-19.23:55:24.876>
    closer = 'vstinner'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2018-06-26.11:55:02.617>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33966
    keywords = ['patch']
    message_count = 4.0
    messages = ['320484', '320525', '320528', '325833']
    nosy_count = 7.0
    nosy_names = ['paul.moore', 'pitrou', 'vstinner', 'tim.golden', 'zach.ware', 'steve.dower', 'davin']
    pr_nums = ['7965', '7966']
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue33966'
    versions = ['Python 3.8']

    @vstinner
    Copy link
    Member Author

    Using my PR 7827, the following command shows a leak of 4 Windows handles per run:

    python -m test -R 3:3 test_multiprocessing_spawn -v -m test.test_multiprocessing_spawn.WithProcessesTestPool.test_traceback

    See also bpo-33929: test_multiprocessing_spawn: WithProcessesTestProcess.test_many_processes() leaks 5 handles on Windows.

    @vstinner vstinner added 3.8 only security fixes stdlib Python modules in the Lib dir OS-windows labels Jun 26, 2018
    @vstinner
    Copy link
    Member Author

    • Pool._repopulate_pool() creates processes with args=(self._inqueue, ...)
    • Pool._inqueue is a multiprocessing.queues.SimpleQueue
    • Process.__init__() of multiprocessing.popen_spawn_win32 calls reduction.dump(process_obj, to_child) which indirectly contains the SimpleQueue
    • A SimpleQueue object contains: self._reader, self._writer = connection.Pipe(duplex=False)
    • dump() indirectly calls reduce_connection() (ex: for SimpleQueue._reader) of multiprocessing.connection
    • reduce_connection() duplicates the pipe handle

    It's unclear to me who is supposed to close the duplicated pipe handles? reduce_connection() creates a "ds = resource_sharer.DupSocket(s)" object, but this object doesn't seem to call CloseHandle()?

    @vstinner
    Copy link
    Member Author

    • dump() indirectly calls reduce_connection() (ex: for SimpleQueue._reader) of multiprocessing.connection
    • reduce_connection() duplicates the pipe handle

    Sorry, it's reduce_pipe_connection(), not reduce_connection().

    It's unclear to me who is supposed to close the duplicated pipe handles? reduce_connection() creates a "ds = resource_sharer.DupSocket(s)" object, but this object doesn't seem to call CloseHandle()?

    reduce_pipe_connection() creates a DupHandle object which duplicates the handle, and it's detach() method closes the handle.

    The race condition is that sometimes the pool terminates a worker (worker() function of multiprocessing.pool) before the worker has time to close the duplicated handle.

    The handle is closed by:

    • rebuild_pipe_connection()
    • dh.detach(), extract:
                    return _winapi.DuplicateHandle(
                        proc, self._handle, _winapi.GetCurrentProcess(),
                        self._access, False, _winapi.DUPLICATE_CLOSE_SOURCE)

    @vstinner
    Copy link
    Member Author

    I give up on that one :-( It doesn't seem easy to fix this corner case without changing the behaviour.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 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 OS-windows stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant