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

bpo-40071: Fix refleak in _functools module. #19172

Merged
merged 1 commit into from Mar 26, 2020
Merged

Conversation

@phsilva
Copy link
Contributor

phsilva commented Mar 26, 2020

@phsilva phsilva requested a review from rhettinger as a code owner Mar 26, 2020
@vstinner vstinner added the skip news label Mar 26, 2020
@vstinner vstinner merged commit b09ae3f into python:master Mar 26, 2020
8 checks passed
8 checks passed
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200326.4 succeeded
Details
bedevere/issue-number Issue number 40071 found
Details
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vstinner

This comment has been minimized.

Copy link
Member

vstinner commented Mar 26, 2020

I merged the PR to bring back Refleaks buildbots to green: the change fix https://bugs.python.org/issue40071

PR 19151 introduced a regression. If a second instance of _functools module is created and then destroyed, _functools_free() will clear kwd_mark. Later, if the first module instance is still used, it will likely crash since kwd_mark is now NULL :-(

C extension modules should not use static global variables, but use a module state.

@phsilva: Do you want to attempt to write a change to add a module state to _functools to store this variable? Maybe @corona10 can help you, or write if you cannot.

@phsilva

This comment has been minimized.

Copy link
Contributor Author

phsilva commented Mar 26, 2020

@phsilva: Do you want to attempt to write a change to add a module state to _functools to store this variable? Maybe @corona10 can help you, or write if you cannot.

Yes, I do. WIll send a PR and ask @corona10 for help if needed.

I suspect this is a little trickier because the module uses kwd_mark in functions that do not receive PyModule*, but will take a further look. Thanks!

@vstinner

This comment has been minimized.

Copy link
Member

vstinner commented Mar 26, 2020

I suspect this is a little trickier because the module uses kwd_mark in functions that do not receive PyModule*, but will take a further look. Thanks!

It's used in lru_cache_make_key() which is called indirectly by:

static PyObject *
lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw)

Hum, maybe this change depends on PEP 573 implementation... which is not written yet! The PEP was just accepted.

@vstinner

This comment has been minimized.

Copy link
Member

vstinner commented Mar 26, 2020

A workaround is to disable _functools_free() for now: never clear kwd_mark. And only reenable _functools_free() once lru_cache_new() will be able to access the module state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.