Author scoder
Recipients Arfrever, amaury.forgeotdarc, eric.snow, loewis, scoder
Date 2011-11-19.08:23:05
SpamBayes Score 1.66533e-16
Marked as misclassified No
Message-id <1321690987.0.0.984903978025.issue13429@psf.upfronthosting.co.za>
In-reply-to
Content
I'm aware that these things happen, that's why I said it. Actually, wouldn't it rather be *correct* for __file__ to be set to the same file path for all modules that an extension module creates in its init function? That would suggest that _Py_ModuleImportContext shouldn't be set to NULL after the first assignment, but instead stay alive until it gets reset by the dynlib loader. If the loader gets invoked recursively later on, it will do the right thing by storing away the old value during the import and restoring it afterwards. So _Py_ModuleImportContext would always point to the path that contains the init function that is currently being executed.

Regarding the lock (which, I assume, is simply reentrant), it's being acquired far up when the import mechanism starts, so the dynlib loader and the init function call are protected.

Note that this does not apply to the reinit case. _PyImport_FindExtensionObject() does not acquire the lock itself (which seems correct), and it can be called directly from imp.init_builtin(), i.e. from user code. Maybe that's why the _Py_PackageContext protocol was not implemented there. That's rather unfortunate, though. I guess the reasoning is that new code that uses this new feature is expected to actually be reentrant, also in parallel, because the module it creates and works on is local to the current thread until the init function terminates. So the import lock is not strictly required here. This does complicate the __file__ feature, though, so the second ("reinit") patch won't work as is. I think the right fix for Python 4 would be to simply pass a context struct into the module init function.

On a related note, I just stumbled over this code in _PyImport_FindExtensionObject():

    else {
        if (def->m_base.m_init == NULL)
            return NULL;
        mod = def->m_base.m_init();
        if (mod == NULL)
            return NULL;
        PyDict_SetItem(PyImport_GetModuleDict(), name, mod);
        Py_DECREF(mod);
    }
    if (_PyState_AddModule(mod, def) < 0) {
        PyDict_DelItem(PyImport_GetModuleDict(), name);
        Py_DECREF(mod);
        return NULL;
    }

If PyDict_SetItem() fails, this is bound to crash. I think it would be worth looking into this mechanism a bit more.
History
Date User Action Args
2011-11-19 08:23:07scodersetrecipients: + scoder, loewis, amaury.forgeotdarc, Arfrever, eric.snow
2011-11-19 08:23:06scodersetmessageid: <1321690987.0.0.984903978025.issue13429@psf.upfronthosting.co.za>
2011-11-19 08:23:06scoderlinkissue13429 messages
2011-11-19 08:23:05scodercreate