Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

bpo-36333: Fix leak in _PyRuntimeState_Fini#12400

Merged
vstinner merged 1 commit into
python:masterpython/cpython:masterfrom
matrixise:bpo-36333-fix_refleak__PyRuntimeState_Finimatrixise/cpython:bpo-36333-fix_refleak__PyRuntimeState_FiniCopy head branch name to clipboard
Mar 19, 2019
Merged

bpo-36333: Fix leak in _PyRuntimeState_Fini#12400
vstinner merged 1 commit into
python:masterpython/cpython:masterfrom
matrixise:bpo-36333-fix_refleak__PyRuntimeState_Finimatrixise/cpython:bpo-36333-fix_refleak__PyRuntimeState_FiniCopy head branch name to clipboard

Conversation

@matrixise

@matrixise matrixise commented Mar 18, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@matrixise matrixise mentioned this pull request Mar 18, 2019
Closed
vstinner
vstinner approved these changes Mar 18, 2019
View reviewed changes

@vstinner vstinner left a comment

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.

LGTM, but I would prefer that @ericsnowcurrently review it since he added _PyRuntime.xidregistry (and its mutex).

@vstinner

vstinner commented Mar 18, 2019

Copy link
Copy Markdown
Member

Python 3.7 is not affected: it doesn't have _PyRuntime.xidregistry.

eamanu
eamanu approved these changes Mar 18, 2019
View reviewed changes

@eamanu eamanu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

ericsnowcurrently
ericsnowcurrently approved these changes Mar 18, 2019
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

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.

LGTM

Thanks for working on this!

@ericsnowcurrently

ericsnowcurrently commented Mar 18, 2019

Copy link
Copy Markdown
Member

FWIW, _PyRuntime.ceval.pending.lock may be leaking too.

@vstinner

vstinner commented Mar 18, 2019

Copy link
Copy Markdown
Member

FWIW, _PyRuntime.ceval.pending.lock may be leaking too.

@matrixise: Can you try to free _PyRuntime.ceval.pending.lock in _PyEval_FiniThreads()?

@matrixise

matrixise commented Mar 18, 2019

Copy link
Copy Markdown
Member Author

@ericsnowcurrently I will check in an other PR. Thanks for the review

@matrixise

matrixise commented Mar 18, 2019

Copy link
Copy Markdown
Member Author

Ok i will do it

@vstinner vstinner merged commit 943395f into python:master Mar 19, 2019
@vstinner

vstinner commented Mar 19, 2019

Copy link
Copy Markdown
Member

I merged this PR, thanks @matrixise.

Oh, there are now too many leaks... I merged https://bugs.python.org/issue36356 PR and I wrote PR #12432 to free the pending lock, to have a better view of remaining leaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

+1 more reviewer

@eamanu eamanu eamanu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@matrixise @vstinner @ericsnowcurrently @eamanu @the-knights-who-say-ni @bedevere-bot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.