Skip to content

bpo-37788: fix reference leak caused by threading._shutdown_locks#15175

Closed
akruis wants to merge 2 commits intopython:mainfrom
akruis:bpo37788-fix-resource-leak
Closed

bpo-37788: fix reference leak caused by threading._shutdown_locks#15175
akruis wants to merge 2 commits intopython:mainfrom
akruis:bpo37788-fix-resource-leak

Conversation

@akruis
Copy link
Copy Markdown

@akruis akruis commented Aug 8, 2019

If a program creates non-daemon threads but does not join them, the global set threading._shutdown_locks accumulates the shutdown locks of the threads.

This pull request adds a test case for non-daemon threads without join. The test must not leak references, if called with python -m test -R: test_threading.

This pull request also fixes the reference leak. The tstate->on_delete-hook now discards per thread shutdown locks from threading._shutdown_locks.

https://bugs.python.org/issue37788

akruis added 2 commits August 8, 2019 08:26
The test must not leak references, if called with
python -m test -R: test_threading
Discard per thread shutdown locks from threading._shutdown_locks when the
thread-state gets deleted.
@vstinner
Copy link
Copy Markdown
Member

I proposed a different approach: PR #15228 adds a __del__() method.

@csabella
Copy link
Copy Markdown
Contributor

csabella commented Feb 6, 2020

@vstinner, should this be closed as the other PR has been merged for this issue? Thanks.

@jussike
Copy link
Copy Markdown

jussike commented Oct 23, 2020

Is someone ( @akruis ?) working with this issue? It's needed because PR #15228 (by @vstinner) has been reverted.

Comment thread Modules/_threadmodule.c
Py_INCREF(shutdown_locks);
data[2] = shutdown_locks;

tstate->on_delete_data = (void *) data;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On_delete_data is no longer just a weak reference to a lock, so the comment in Include/cpython/pystate.h is out of date.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented May 13, 2021

PR #26103 is an arguably simpler fix for this (it doesn't complicate the C side).

@pitrou
Copy link
Copy Markdown
Member

pitrou commented May 15, 2021

PR #26103 is merged, closing this.

@pitrou pitrou closed this May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants