[Python-Dev] ANSI strict aliasing and Python

[Tim Peters]

[Andrew MacIntyre]
>> BTW, the following sequence of tests causes a core dump from an
>> assertion failure in test_enumerate on EMX which I haven't been able
>> to replicate on FreeBSD :-(
>>
>> test_importhooks
>> test_re
>> test_glob
>> test_parser
>> test_enumerate

This one's a bitch.  For me on Win98SE, it may or may not be related to that
the parser module is in a separate DLL.

Here's the start of initparser():

PyMODINIT_FUNC
initparser(void)
{
    PyObject *module, *copyreg;

    PyST_Type.ob_type = &PyType_Type;
    module = Py_InitModule("parser", parser_functions);

    if (parser_error == 0)
        parser_error = PyErr_NewException("parser.ParserError", NULL,
                       NULL);

    if ((parser_error == 0)
        || (PyModule_AddObject(module, "ParserError", parser_error) != 0)) {
        /* caller will check PyErr_Occurred() */
        return;
    }


It's skating on the edge a little in "the usual" case:  the file static
parser_error starts life at NULL, so PyErr_NewException gets called, and
PyModule_AddObject *transfers* ownership of the (sole) reference, from the
parser_error static to the module dict.  This is a little dicey because
functions all over parsermodule.c use parser_error, but the module proper
doesn't own a reference to parser_error.

But that's probably fine so far as it goes, because gc isn't aware of the
parser_error file static (so the file static doesn't look like an "extra
reference" to it, so far as gc is concerned), and it's hard to imagine how
functions in this module could get invoked if the true owner (the module
dict) went away.

Things go to hell in the sequence of tests that Andrew found, though,
because initparser() is called *twice*.  It's called once while running
test_importhooks, and a second time while running test_parser.  The second
time it's called, parser_error still has the value it got from
PyErr_NewException in the first call.  So PyErr_NewException isn't called
again, but PyModule_AddObject is, and ownership of the (shared) parser_error
object is effectively transferred from the old module dict to the new module
dict -- but both dicts still contain it!  That's the cause of the gc
assertion:  more containers point to parser_error than can be accounted for
by parser_error->ob_refcnt (which, on the second call to initparser(),
starts at and remains 1).

The simplest fix is to incref parser_error before calling
PyModule_AddObject.  Then each module dict created will own its reference,
and the file static parser_error will own a reference too.  This will make
parser_error immortal (as is the case for any module with a file static that
owns a reference to an object, and doesn't have a module cleanup function
that gets called at the right times).

1. Is that a correct fix?
2. If so, is this important enough to justify another release candidate?

I think this release has gotten messy enough that we're going to need a
branch.  I'll create one.