This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author arigo
Recipients arigo, eric.snow, ncoghlan, petr.viktorin
Date 2019-03-02.07:33:48
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1551512028.73.0.931816514669.issue36124@roundup.psfhosted.org>
In-reply-to
Content
PyModule_GetState() requires having the module object that corresponds to the given interpreter state.  I'm not sure how a C extension module is supposed to get its own module object corresponding to the current interpreter state, without getting it from the caller in some way.

The mess in cffi's call_python.c would be much reduced if we had bpo-36124 (fixed to call Py_CLEAR(), see comment in bpo-36124).

If you want to point out a different approach that might work too, that's OK too.  It's just that the current approach was arrived at after multiple generations of crash reports, which makes me uneasy about changing it in more subtle ways than just killing it in favor of a careful PyInterpreterState_GetDict().

If you want to see details of the current hacks, I can explain https://bitbucket.org/cffi/cffi/src/d765c36df047cf9d5e766777049c4107e1f4cb00/c/call_python.c :

The goal is that we are given a finite (but unknown at compile-time) number of 'externpy' data structures, and for each pair (externpy, interp) the user can assign a callable 'PyObject *'.  The annoying part of the logic is that we have a C-exposed callback function (line 204) which is called with a pointer to one of these 'externpy' structures, and we need to look up the right 'PyObject *' to call.

At line 255 we just got the GIL and need to check if the 'PyThreadState_GET()->interp' is equal to the one previously seen (an essential optimization: we can't do complicated logic in the fast path).  We hack by checking for 'interp->modules' because that's a PyObject.  The previous time this code was invoked, we stored a reference to 'interp->modules' in the C structure 'externpy', with an incref.  So this fast-path pointer comparison is always safe (no freed object whose address can be accidentally reused).  This test will quickly pass if this function is called in the same 'interp' many times in a row.

The slow path is in _update_cache_to_call_python(), which calls _get_interpstate_dict(), whose only purpose is to return a dictionary that depends on 'interp'.  Note how we need to be very careful about various cases, like shutdown.  _get_interpstate_dict() can fail and return NULL, but it cannot give a fatal error.  That's why we couldn't call, say, PyImport_GetModuleDict(), because this gives a fatal error if 'interp' is being shut down at the moment.

Overall, the logic uses both 'interp->modules' and 'interp->builtins'.  The 'modules' is used only for the pointer equality check, because that's an object that is not supposed to be freed until the very last moment.  The 'builtins' is used to store the special name "__cffi_backend_extern_py" in it, because we can't store that in 'interp->modules' directly without crashing various 3rd-party Python code if this special key shows up in 'sys.modules'.  The value corresponding to this special name is a dictionary {PyLong_FromVoidPtr(externpy): infotuple-describing-the-final-callable}.
History
Date User Action Args
2019-03-02 07:33:48arigosetrecipients: + arigo, ncoghlan, petr.viktorin, eric.snow
2019-03-02 07:33:48arigosetmessageid: <1551512028.73.0.931816514669.issue36124@roundup.psfhosted.org>
2019-03-02 07:33:48arigolinkissue36124 messages
2019-03-02 07:33:48arigocreate