diff -r c28e07377b03 Lib/test/test_zipimport.py --- a/Lib/test/test_zipimport.py Tue Jan 21 21:12:13 2014 -0500 +++ b/Lib/test/test_zipimport.py Wed Jan 22 02:25:55 2014 -0800 @@ -395,10 +395,15 @@ def setUp(self): zipimport._zip_directory_cache.clear() zipimport._zip_stat_cache.clear() + # save sys.modules so we can unimport everything done by our tests. + self._sys_modules_orig = dict(sys.modules) ImportHooksBaseTestCase.setUp(self) def tearDown(self): ImportHooksBaseTestCase.tearDown(self) + # The closest we can come to un-importing our zipped up test modules. + sys.modules.clear() + sys.modules.update(self._sys_modules_orig) if os.path.exists(TEMP_ZIP): os.remove(TEMP_ZIP) @@ -415,13 +420,19 @@ self.assertTrue(os.path.exists(zipfile_path)) sys.path.insert(0, zipfile_path) + testpack_testmod = TESTPACK + "." + TESTMOD # Import something out of the zipfile and confirm it is correct. - testmod = __import__(TESTPACK + "." + TESTMOD, + testmod = __import__(testpack_testmod, globals(), locals(), ["__dummy__"]) self.assertEqual(testmod.test_value, 38) + del testmod + del sys.modules[TESTPACK] + del sys.modules[testpack_testmod] # Import something else out of the zipfile and confirm it is correct. ziptest_b = __import__("ziptest_b", globals(), locals(), ["test_value"]) self.assertEqual(ziptest_b.test_value, 42) + del ziptest_b + del sys.modules["ziptest_b"] # Truncate and fill the zip file with non-zip garbage. with io.open(zipfile_path, "rb") as orig_zip_file: @@ -433,6 +444,14 @@ with self.assertRaises(ImportError): ziptest_a = __import__("ziptest_a", globals(), locals(), ["test_value"]) + # The code path used by the __import__ call is different than + # that used by import statements. + with self.assertRaises(ImportError): + import ziptest_a + with self.assertRaises(ImportError): + from ziptest_a import test_value + with self.assertRaises(ImportError): + exec("from {} import {}".format(TESTPACK, TESTMOD)) # Now lets make it a valid zipfile that has some garbage at the start. # This alters all of the offsets within the file @@ -440,6 +459,12 @@ new_zip_file.write(b"X"*1991) # The year Python was created. new_zip_file.write(orig_zip_file_contents) + # importing a submodule triggers a different import code path. + exec("import " + testpack_testmod) + self.assertEqual(getattr(locals()[TESTPACK], TESTMOD).test_value, 38) + exec("from {} import {}".format(TESTPACK, TESTMOD)) + self.assertEqual(locals()[TESTMOD].test_value, 38) + # Now that the zip file has been "restored" to a valid but different # zipfile the zipimporter should *successfully* re-read the new zip # file's end of file central index and be able to import from it again. diff -r c28e07377b03 Modules/zipimport.c --- a/Modules/zipimport.c Tue Jan 21 21:12:13 2014 -0500 +++ b/Modules/zipimport.c Wed Jan 22 02:25:55 2014 -0800 @@ -130,6 +130,10 @@ } if (path != NULL) { PyObject *files; + /* TODO(gps): why does this lookup and store via path into + * zip_directory_cache which _could_ still have trailing + * "prefix/spam/beans.py" on it instead of buf??? + * NOTE: I've never seen that happen. When would it? */ files = PyDict_GetItemString(zip_directory_cache, path); if (files == NULL) { PyObject *zip_stat = NULL; @@ -165,9 +169,6 @@ } Py_XDECREF(zip_stat); } - else - Py_INCREF(files); - self->files = files; } else { PyErr_SetString(ZipImportError, "not a Zip file"); @@ -197,22 +198,11 @@ return 0; } -/* GC support. */ -static int -zipimporter_traverse(PyObject *obj, visitproc visit, void *arg) -{ - ZipImporter *self = (ZipImporter *)obj; - Py_VISIT(self->files); - return 0; -} - static void zipimporter_dealloc(ZipImporter *self) { - PyObject_GC_UnTrack(self); Py_XDECREF(self->archive); Py_XDECREF(self->prefix); - Py_XDECREF(self->files); Py_TYPE(self)->tp_free((PyObject *)self); } @@ -292,6 +282,7 @@ char *subname, path[MAXPATHLEN + 1]; int len; struct st_zip_searchorder *zso; + PyObject *files; subname = get_subname(fullname); @@ -299,9 +290,23 @@ if (len < 0) return MI_ERROR; + files = PyDict_GetItem(zip_directory_cache, self->archive); + if (files == NULL) { + /* Some scoundrel has cleared zip_directory_cache out from + * beneath us. Try repopulating it once before giving up. */ + char *unused_archive_name; + FILE *fp = safely_reopen_archive(self, &unused_archive_name); + if (fp == NULL) + return MI_ERROR; + fclose(fp); + files = PyDict_GetItem(zip_directory_cache, self->archive); + if (files == NULL) + return MI_ERROR; + } + for (zso = zip_searchorder; *zso->suffix; zso++) { strcpy(path + len, zso->suffix); - if (PyDict_GetItemString(self->files, path) != NULL) { + if (PyDict_GetItemString(files, path) != NULL) { if (zso->type & IS_PACKAGE) return MI_PACKAGE; else @@ -456,7 +461,7 @@ #ifdef ALTSEP char *p, buf[MAXPATHLEN + 1]; #endif - PyObject *toc_entry, *data; + PyObject *toc_entry, *data, *files; Py_ssize_t len; if (!PyArg_ParseTuple(args, "s:zipimporter.get_data", &path)) @@ -485,7 +490,13 @@ if (fp == NULL) return NULL; - toc_entry = PyDict_GetItemString(self->files, path); + files = PyDict_GetItem(zip_directory_cache, self->archive); + if (files == NULL) { + /* This should never happen as safely_reopen_archive() should + * have repopulated zip_directory_cache if needed. */ + return NULL; + } + toc_entry = PyDict_GetItemString(files, path); if (toc_entry == NULL) { PyErr_SetFromErrnoWithFilename(PyExc_IOError, path); fclose(fp); @@ -512,7 +523,7 @@ zipimporter_get_source(PyObject *obj, PyObject *args) { ZipImporter *self = (ZipImporter *)obj; - PyObject *toc_entry; + PyObject *toc_entry, *files; FILE *fp; char *fullname, *subname, path[MAXPATHLEN+1], *archive; int len; @@ -546,7 +557,13 @@ if (fp == NULL) return NULL; - toc_entry = PyDict_GetItemString(self->files, path); + files = PyDict_GetItem(zip_directory_cache, self->archive); + if (files == NULL) { + /* This should never happen as safely_reopen_archive() should + * have repopulated zip_directory_cache if needed. */ + return NULL; + } + toc_entry = PyDict_GetItemString(files, path); if (toc_entry != NULL) { PyObject *data = get_data(fp, archive, toc_entry); fclose(fp); @@ -666,10 +683,9 @@ PyObject_GenericGetAttr, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | - Py_TPFLAGS_HAVE_GC, /* tp_flags */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ zipimporter_doc, /* tp_doc */ - zipimporter_traverse, /* tp_traverse */ + 0, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ 0, /* tp_weaklistoffset */ @@ -686,7 +702,7 @@ (initproc)zipimporter_init, /* tp_init */ PyType_GenericAlloc, /* tp_alloc */ PyType_GenericNew, /* tp_new */ - PyObject_GC_Del, /* tp_free */ + 0, /* tp_free */ }; @@ -830,11 +846,13 @@ fclose(fp); return NULL; } - Py_XDECREF(self->files); /* free the old value. */ - self->files = files; } Py_DECREF(stat_now); - } /* stat succeeded */ + } else { + if (Py_VerboseFlag) + PySys_WriteStderr("# zipimport: os.fstat failed on the " + "open %s file.\n", archive); + } return fp; } @@ -1236,12 +1254,18 @@ static time_t get_mtime_of_source(ZipImporter *self, char *path) { - PyObject *toc_entry; + PyObject *toc_entry, *files; time_t mtime = 0; Py_ssize_t lastchar = strlen(path) - 1; char savechar = path[lastchar]; path[lastchar] = '\0'; /* strip 'c' or 'o' from *.py[co] */ - toc_entry = PyDict_GetItemString(self->files, path); + files = PyDict_GetItem(zip_directory_cache, self->archive); + if (files == NULL) { + /* This should never happen as safely_reopen_archive() from + * our only caller repopulated zip_directory_cache if needed. */ + return 0; + } + toc_entry = PyDict_GetItemString(files, path); if (toc_entry != NULL && PyTuple_Check(toc_entry) && PyTuple_Size(toc_entry) == 8) { /* fetch the time stamp of the .py file for comparison @@ -1304,7 +1328,7 @@ return NULL; for (zso = zip_searchorder; *zso->suffix; zso++) { - PyObject *code = NULL; + PyObject *code = NULL, *files; strcpy(path + len, zso->suffix); if (Py_VerboseFlag > 1) @@ -1312,7 +1336,14 @@ PyString_AsString(self->archive), SEP, path); - toc_entry = PyDict_GetItemString(self->files, path); + files = PyDict_GetItem(zip_directory_cache, self->archive); + if (files == NULL) { + /* This should never happen as safely_reopen_archive() should + * have repopulated zip_directory_cache if needed; and the GIL + * is being held. */ + return NULL; + } + toc_entry = PyDict_GetItemString(files, path); if (toc_entry != NULL) { time_t mtime = 0; int ispackage = zso->type & IS_PACKAGE;