classification
Title: Refcount issues in import
Type: resource usage Stage: resolved
Components: Interpreter Core Versions: Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, eric.snow, indygreg, ncoghlan, petr.viktorin, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2020-10-10 11:14 by serhiy.storchaka, last changed 2021-09-28 01:02 by indygreg. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22632 merged serhiy.storchaka, 2020-10-10 11:17
Messages (7)
msg378384 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-10-10 11:14
There is a reference leak in import_add_module(). PyDict_GetItemWithError() returns a borrowed reference, but PyObject_GetItem() return a non-borrowed reference. If sys.modules is not a dict, there is a reference leak. import_add_module() and several other function which return a borrowed reference should be made returning a non-borrowed reference, because there are no guaranties that general mapping keeps reference to value.

It is still not guarantee correctness of PyImport_AddModuleObject().
msg384943 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-01-12 14:43
New changeset 4db8988420e0a122d617df741381b0c385af032c by Serhiy Storchaka in branch 'master':
bpo-41994: Fix refcount issues in Python/import.c (GH-22632)
https://github.com/python/cpython/commit/4db8988420e0a122d617df741381b0c385af032c
msg384947 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-01-12 14:47
Thank you Petr!
msg402688 - (view) Author: Gregory Szorc (indygreg) * Date: 2021-09-27 03:43
While testing 3.10.0rc2, I noticed that commit 4db8988420e0a122d617df741381b0c385af032c removed _PyImport_FindExtensionObject() from libpython. This breaks both PyOxidizer and py2exe, which rely on this function for a custom implementation of Loader.create_module() to enable loading Windows DLLs from memory.

It can probably be worked around. But I wanted to note it here in case it was an unwanted regression.
msg402696 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-09-27 08:55
Please don't use private API. The underscore at the beginning marks the function as private.
(This has been the case for a long time; in 3.10 it is finally officially documented: https://docs.python.org/3.10/c-api/stable.html#stable )

It has been removed in alpha5; I'm afraid rc2 is too late to put it back. (Especially since we'd need to put it back as public API, as CPython doesn't need it any more.)


Please, if you see the need for any other private APIs, could you file bugs to make them public (or better, to support the use case they're needed for)? If they are useful, they should have documentation, tests, and better backwards compatibility expecations.
msg402698 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-09-27 09:59
_PyImport_FindExtensionObject is a private API. It was added in 3.3 because import.c and importdl.c needed to share code. Since 3.5 it was only used in import.c, so there is no longer need to expose it. It was removed in 3.10 because there was an issue with this API: it returned a borroved reference which could be invalid at the time of returning.

I can restore and fix _PyImport_FindExtensionObject in 3.10, but is not it too later to do after 3.10.0rc2?

If it is essential to you, propose to add a new public API (perhaps better designed).
msg402755 - (view) Author: Gregory Szorc (indygreg) * Date: 2021-09-28 01:02
I didn't want to derail this old issue too much. So I filed issue45307 to track a solution.
History
Date User Action Args
2021-09-28 01:02:00indygregsetmessages: + msg402755
2021-09-27 09:59:02serhiy.storchakasetmessages: + msg402698
2021-09-27 08:55:14petr.viktorinsetmessages: + msg402696
2021-09-27 03:43:15indygregsetnosy: + indygreg
messages: + msg402688
2021-01-12 14:47:15serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg384947

stage: patch review -> resolved
2021-01-12 14:43:43petr.viktorinsetnosy: + petr.viktorin
messages: + msg384943
2020-10-10 11:17:14serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request21609
2020-10-10 11:14:48serhiy.storchakacreate