[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Soujanya. Pasam added the comment: I see test_cpickle failing with NPE while running the tests with IBM Java. Exception Stack Trace

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: Thanks for digging deeply and fixing this! -- ___ Python tracker ___ ___

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: > That seems fine except you might as well just put the GetWithError prototype > in the header file. Yes, of course. It was temporary placed here to avoid rebuilding all sources. Thank you Barry and Benjamin for investigating this issue. --

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Roundup Robot added the comment: New changeset 81dba6e392c3 by Serhiy Storchaka in branch '2.7': Issue #25698: Importing module if the stack is too deep no longer replaces https://hg.python.org/cpython/rev/81dba6e392c3 New changeset 1c2de3b0a474 by Serhiy Storchaka in branch '3.5': Issue

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Benjamin Peterson added the comment: That seems fine except you might as well just put the GetWithError prototype in the header file. -- ___ Python tracker

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Agreed. Here is a patch that makes PyImport_AddModule to use PyDict_GetWithError and fail earlier. I think it should be applied in 3.x too. -- Added file: http://bugs.python.org/file41846/issue25698_fix_add_module.patch

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Benjamin, could you please make a review? -- ___ Python tracker ___ ___

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Benjamin Peterson added the comment: I don't think this patch is the best way. The correct way is to have and use PyDict_GetWithError. I wouldn't be surprised if you could crash Python under this patch by putting some function with an import in __cmp__ in sys.modules. On Sun, Feb 7, 2016, at

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Changes by Serhiy Storchaka : -- priority: normal -> high stage: -> patch review ___ Python tracker ___

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: One solution is to skip tests caused deep recursion. The better solution would be to fix import machinery to not produce empty modules. -- keywords: +patch Added file: http://bugs.python.org/file41748/issue25698_skip_tests.patch

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Here is a patch that makes import machinery to not left empty module in case of recursion error. I prefer this solution. -- Added file: http://bugs.python.org/file41750/issue25698_fix_remove_module.patch ___

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: On Dec 05, 2015, at 07:03 AM, Benjamin Peterson wrote: >The underlying issue is classic: PyDict_SetItem returns NULL if the stack is >too deep. This confuses import. Thanks for digging into this Benjamin. I'm not able to investigate further atm, but could

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Benjamin Peterson added the comment: Err, sorry, it's actually PyDict_GetItem which is the problem. Basically if you try to import with a very deep stack, doing PyDict_GetItem on sys.modules can return NULL (fro copy_reg). That confuses import a lot. I haven't exactly figured out why those

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Benjamin Peterson added the comment: Err, sorry, it's actually PyDict_GetItem which is the problem. Basically if you try to import with a very deep stack, doing PyDict_GetItem on sys.modules can return NULL (fro copy_reg). That confuses import a lot. I haven't exactly figured out why those

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Thank you Benjamin! This stably reproduce the issue. Just wondering how you found this minimal sequence? -- ___ Python tracker

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Benjamin Peterson added the comment: I can reproduce with % ./python Lib/test/regrtest.py test___all__ test_memoryio test_zipimport test_cpickle The underlying issue is classic: PyDict_SetItem returns NULL if the stack is too deep. This confuses import. Not new, so not a release blocker.

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: At the problematic breakpoint mentioned in msg255630, copy_reg is nearly empty. (Pdb) sys.modules['copy_reg'] (Pdb) dir(sys.modules['copy_reg']) ['__builtins__', '__doc__', '__file__', '__name__', '__package__'] (Pdb) sys.modules['copy_reg'].__file__

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Was it empty before running any test_cpickle tests? Could you find after what test it becomes empty? -- ___ Python tracker

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: Well, one thing I noticed is that init_stuff() in cPickle.c can leak the copy_reg module object, if any of the subsequent PyObject_GetAttr*() calls fail. They return -1 without decref'ing copyreg. Probably not a horrible leak since it's probably rare, but

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: On Dec 01, 2015, at 06:26 PM, Barry A. Warsaw wrote: >I'll note one other thing. Doko mentioned that in our build environment, >cPickle is built in, not an extension. Which may be relevant because in a from-hg-head build of Python 2.7, this problem doesn't

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: This just gets weirder. I've narrowed it down to running this command: build-static/python Lib/test/regrtest.py test_doctest test_cpickle fails every time. What's even weirder is that I hacked regrtest to print some useful information before and after each

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Thanks Barry. Running test_cpickle after test_doctest I now can reproduce the problem in a from-hg-head build of Python 2.7. -- ___ Python tracker

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: On Dec 01, 2015, at 06:47 PM, Serhiy Storchaka wrote: >Thanks Barry. Running test_cpickle after test_doctest I now can reproduce the >problem in a from-hg-head build of Python 2.7. That's interesting because I can't! (Neither from the 2.7.11rc1 tarball.)

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: Ah, sorry, I missed and mislead you. My tests were specially hacked to simulate a bug (sys.modules['copy_reg'] = object()). No, actually I can't reproduce it. -- ___ Python tracker

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Barry A. Warsaw added the comment: I added cPickle to Modules/Setup.local and rebuilt from hg. cPickle's a built-in but I still can't reproduce the problem. -- ___ Python tracker

[issue25698] The copy_reg module becomes unexpectedly empty in test_cpickle

Serhiy Storchaka added the comment: http://buildbot.python.org/all/builders/PPC64LE%20Fedora%202.7/builds/164/steps/test/logs/stdio Failed a number of tests that directly or indirectly use copy_reg. First failed test is test_cpickle. -- title: test regressions introduced with the fix