Skip to content

Conversation

@phsilva
Copy link
Contributor

@phsilva phsilva commented Mar 26, 2020

@vstinner
Copy link
Member

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
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
Copy link
Member

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
Copy link
Member

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

Development

Successfully merging this pull request may close these issues.

4 participants