[issue41631] _ast module: get_global_ast_state() doesn't work with Mercurial lazy import

[STINNER Victor]

STINNER Victor <vstin...@python.org> added the comment:

At commit ac46eb4ad6662cf6d771b20d8963658b2186c48c ("bpo-38113: Update the 
Python-ast.c generator to PEP384 (gh-15957)"):
---------------
static struct PyModuleDef _astmodule = {
    PyModuleDef_HEAD_INIT,
    "_ast",
    NULL,
    sizeof(astmodulestate),
    NULL,
    NULL,
    astmodule_traverse,
    astmodule_clear,
    astmodule_free,
};

#define astmodulestate_global ((astmodulestate 
*)PyModule_GetState(PyState_FindModule(&_astmodule)))

PyMODINIT_FUNC
PyInit__ast(void)
{
    PyObject *m;
    if (!init_types()) return NULL;
    m = PyState_FindModule(&_astmodule);
    ...
    return m;
}
---------------

=> I investigated bpo-41194 crash again. It seems like the root issue is more 
than PyInit__ast() returns a *borrowed* reference, rather than a strong 
reference.

=> The _ast module can only be loaded once, astmodule_free() and 
astmodule_clear() are not called at Python exit.


At commit 91e1bc18bd467a13bceb62e16fbc435b33381c82 ("bpo-41194: The _ast module 
cannot be loaded more than once (GH-21290)"):
---------------
static astmodulestate global_ast_state;

static astmodulestate *
get_ast_state(PyObject *Py_UNUSED(module))
{
    return &global_ast_state;
}

#define get_global_ast_state() (&global_ast_state)

static struct PyModuleDef _astmodule = {
    PyModuleDef_HEAD_INIT,
    .m_name = "_ast",
    .m_size = -1,
    .m_traverse = astmodule_traverse,
    .m_clear = astmodule_clear,
    .m_free = astmodule_free,
};

PyMODINIT_FUNC
PyInit__ast(void)
{
    PyObject *m = PyModule_Create(&_astmodule);
    if (!m) {
        return NULL;
    }
    astmodulestate *state = get_ast_state(m);
    ...
}
---------------

* bpo-41194 => ok, PyInit__ast() is fine
* bpo-41631 => ok: PyAST_Check() doesn't import the module, it uses a global 
state.
* bpo-41261 => ERROR: This implementation allows to create two instances of the 
_ast module, but also to unload one instance (call astmodule_free), because 
_astmodule.m_size = -1.


At commit b1cc6ba73a51d5cc3aeb113b5e7378fb50a0e20a ("bpo-41194: Convert _ast 
extension to PEP 489 (GH-21293)"):
---------------
static astmodulestate*
get_global_ast_state(void)
{
    ...
    PyObject *module = PyImport_GetModule(name);
    if (module == NULL) {
        if (PyErr_Occurred()) {
            return NULL;
        }
        module = PyImport_Import(name);
        if (module == NULL) {
            return NULL;
        }
    }
    ...
    astmodulestate *state = get_ast_state(module);
    Py_DECREF(module);
    return state;
}

static struct PyModuleDef _astmodule = {
    PyModuleDef_HEAD_INIT,
    .m_name = "_ast",
    .m_size = sizeof(astmodulestate),
    .m_slots = astmodule_slots,
    .m_traverse = astmodule_traverse,
    .m_clear = astmodule_clear,
    .m_free = astmodule_free,
};

PyMODINIT_FUNC
PyInit__ast(void)
{
    return PyModuleDef_Init(&_astmodule);
}
---------------

* bpo-41631 => ERROR: PyAST_Check() imports "_ast" module

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue41631>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com