classification
Title: PyImport_GetModule() can return partially-initialized module
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: nanjekyejoannah Nosy List: Valentyn Tymofieiev, brett.cannon, eric.snow, nanjekyejoannah, ncoghlan, p-ganssle, pablogsal, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2019-02-08 17:13 by pitrou, last changed 2020-01-21 14:14 by nanjekyejoannah. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15057 merged nanjekyejoannah, 2019-07-31 15:51
Messages (9)
msg335097 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2019-02-08 17:13
PyImport_GetModule() returns whatever is in sys.modules, even if the module is still importing and therefore only partially initialized.

One possibility is to reuse the optimization already done in PyImport_ImportModuleLevelObject():

        /* Optimization: only call _bootstrap._lock_unlock_module() if
           __spec__._initializing is true.
           NOTE: because of this, initializing must be set *before*
           stuffing the new module in sys.modules.
         */
        spec = _PyObject_GetAttrId(mod, &PyId___spec__);
        if (_PyModuleSpec_IsInitializing(spec)) {
            PyObject *value = _PyObject_CallMethodIdObjArgs(interp->importlib,
                                            &PyId__lock_unlock_module, abs_name,
                                            NULL);
            if (value == NULL) {
                Py_DECREF(spec);
                goto error;
            }
            Py_DECREF(value);
        }
        Py_XDECREF(spec);

Issue originally mentioned in issue34572.
msg335100 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-02-08 17:18
Yeah, that makes sense.
msg351847 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-09-11 12:47
New changeset 37c22206981f52ae35c28b39f7530f8438afbfdb by Brett Cannon (Joannah Nanjekye) in branch 'master':
bpo-35943: Prevent PyImport_GetModule() from returning a partially-initialized module (GH-15057)
https://github.com/python/cpython/commit/37c22206981f52ae35c28b39f7530f8438afbfdb
msg356920 - (view) Author: Valentyn Tymofieiev (Valentyn Tymofieiev) Date: 2019-11-18 22:11
Do we plan to backport the change by nanjekyejoannah to 3.7 branch?
msg356980 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-11-19 17:37
I've assigned this to Joannah to decide if she wants to backport this.
msg357201 - (view) Author: Valentyn Tymofieiev (Valentyn Tymofieiev) Date: 2019-11-21 19:30
Thanks. Is it possible that this issue and  https://bugs.python.org/issue38884 are duplicates?
msg360056 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python committer) Date: 2020-01-15 13:59
The changes required to successfully do this backport are many and affect critical areas. I am not in a hurry to do this. If anyone else wants to take this up quickly, please do.
msg360075 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-01-15 21:27
> The changes required to successfully do this backport are many and affect critical areas. I am not in a hurry to do this. If anyone else wants to take this up quickly, please do.

Do you mean that there is a risk that the backport introduces a regression in another part of the code? If yes, I would suggest to not backport the change to *stable* branches.

People survived with bug. Do you really *have to* backport the fix?

Note: this issue is closed. If you consider to backport it, I suggest to reopen the issue.
msg360399 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python committer) Date: 2020-01-21 14:14
> Do you mean that there is a risk that the backport introduces a regression in another part of the code? If yes, I would suggest to not backport the change to *stable* branches.

My worry are the many changes that are required to ceval to make this back port work. Not that I think we can not successfully backport things. we can.
History
Date User Action Args
2020-01-21 14:14:27nanjekyejoannahsetmessages: + msg360399
2020-01-15 21:27:52vstinnersetmessages: + msg360075
2020-01-15 13:59:48nanjekyejoannahsetmessages: + msg360056
2019-11-21 19:30:35Valentyn Tymofieievsetmessages: + msg357201
2019-11-19 17:37:29brett.cannonsetassignee: nanjekyejoannah

messages: + msg356980
nosy: + nanjekyejoannah
2019-11-18 22:11:12Valentyn Tymofieievsetnosy: + Valentyn Tymofieiev
messages: + msg356920
2019-09-11 12:48:02brett.cannonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-09-11 12:47:42brett.cannonsetmessages: + msg351847
2019-07-31 15:51:07nanjekyejoannahsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request14806
2019-02-22 17:27:35pitrousetnosy: + vstinner
2019-02-08 18:56:13p-gansslesetnosy: + p-ganssle
2019-02-08 17:19:40pitrousetnosy: + pablogsal
2019-02-08 17:18:15eric.snowsetmessages: + msg335100
2019-02-08 17:13:37pitroucreate