[Python-Dev] Python/import.c

[Thomas Heller]

"Fred L. Drake, Jr." <fdrake at acm.org> writes:

> On Monday 07 June 2004 01:21 pm, Jiwon Seo wrote:
>  > It looks like it's because even if the package failed to be loaded, it
>  > should be added to its parent module dict, IMHO.
>  > The comment in the import.c::add_submodule describes the behavior. I
>  > don't know how it could be useful, but I think it's the way import
>  > behaves.
>
> This is to support caching; if some module in the "foo" package imports the 
> "bar" module, and there isn't a "foo.bar" module, sys.modules grows a 
> "foo.bar": None entry so that further attempts to import sys within the "foo" 
> package don't have to do as much work to know that there isn't one, and can 
> fall back to a top-level "bar" module (which can still fail if there isn't 
> one).

I knew this already ;-), but it doesn't apply to my question.  The code
I quoted was commited in import.c rev 2.82 (6 years, 9 months ago), but
adding dummy entries to sys.modules was added in rev 2.85.

Here's the code again, with the refcount bug fixed (which I already
applied in CVS):
Thomas Heller <theller at python.net> writes:

> In the context of http://www.python.org/sf/845802 I'm looking at the
> code in Python/import.c, function load_package().
>
> The latter part of the function is this code, which tries to import the
> __init__.py file, after building the module object in 'm', and
> initializing it's __file__ and __path__ attributes:
>
>         fdp = find_module(name, "__init__", path, buf, sizeof(buf), &fp, NULL);
>         if (fdp == NULL) {
>                 if (PyErr_ExceptionMatches(PyExc_ImportError)) {
>                         PyErr_Clear();
                          Py_INCREF(m);
>                 }
>                 else
>                         m = NULL;
>                 goto cleanup;
>         }
>         m = load_module(name, fp, buf, fdp->type, NULL);
>         if (fp != NULL)
>                 fclose(fp);
>   cleanup:
>         Py_XDECREF(path);
>         Py_XDECREF(file);
>         return m;
> }
>
>
> I do not understand why the function doesn't fail when find_module
> returns NULL because of an PyExc_ImportError.  For the above mentioned
> bug, when __init__.py is a directory instead of a file, find_module
> returns NULL.

As far as I can tell, the code in the 'if (fdp == NULL)' part is never
executed by the whole test suite, and I believe the refcount bug would
have been found much earlier otherwise.  This code *is* executed (and
the bug was triggered, at least in a debug build) when you try to import
a package where '__init__.py' is a directory instead of a file.

I believe the code should read:

>         fdp = find_module(name, "__init__", path, buf, sizeof(buf), &fp, NULL);
>         if (fdp == NULL) {
>                 m = NULL;
>                 goto cleanup;
>         }
>         m = load_module(name, fp, buf, fdp->type, NULL);
>         if (fp != NULL)
>                 fclose(fp);
>   cleanup:
>         Py_XDECREF(path);
>         Py_XDECREF(file);
>         return m;
> }

I'll upload a patch.

Thomas