bpo-30697: fix PyErr_NormalizeException() when no memory

[xdegaye]

See the corresponding post on bpo-30697.

https://bugs.python.org/issue30697

[pitrou]

Besides the comments, it would be nice to add some tests.

[pitrou]

Is it right to use a static variable instead of a threadstate-attached variable? What if the GIL is released somewhere in-between and the wrong thread gets the recursion error (or, worse, triggers a fatal error)?

[xdegaye]

You are right of course. I will fix that.

[pitrou]

A nit, but you can use PyErr_SetString if you don't use the formatting capabilities.

[serhiy-storchaka]

Or even PyErr_SetObject() with pre-created static string object (this is used in some critical cases).

[xdegaye]

The instantiation of the RecursionError object itself fails anyway upon its normalization when there is no memory (and a memory error is reported to the user and not the recursion error). So it seems there is no advantage in using PyErr_SetObject() with a pre-created static string object over PyErr_SetString() here.

[xdegaye]

The issues bpo-30695 and bpo-30696 must be fixed before a test with memory exhaustion can be added. A test raising _testcapi.RecursingInfinitelyError can be added at that time.

About the third test, the one from PR #1981 named test_recursion_normalizing_exception(), it is not clear if it should also be added here.

[pitrou]

The issues bpo-30695 and bpo-30696 must be fixed before a test with memory exhaustion can be added.

Should we wait for these issues to be fixed first, then?

About the third test, the one from PR #1981 named test_recursion_normalizing_exception(), it is not clear if it should also be added here.

Why not?

[xdegaye]

Should we wait for these issues to be fixed first, then?

This issue is initially about memory exhaustion, so there must be a test with memory errors. I realize now that bpo-30696 is a separate issue related to the interactive interpeter so we only need to wait for bpo-30695 to be fixed in order to build this test case.

The next commit will add the two other test cases (i.e also the one from PR #1981).

[xdegaye]

Interestingly after the last change in the test case in the last commit, the Appveyor build of the .c files fails now with the following error:

  sysmodule.c
..\Python\sysmodule.c(1968): error C2065: 'PYTHONFRAMEWORK': undeclared identifier [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
..\Python\sysmodule.c(1968): warning C4047: 'function': 'const char *' differs in levels of indirection from 'int' [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
..\Python\sysmodule.c(1968): warning C4024: 'PyUnicode_FromString': different types for formal and actual parameter 1 [C:\projects\cpython\PCbuild\pythoncore.vcxproj]
  thread.c
  traceback.c
  dl_nt.c
Command exited with code 1

It is sad that the results of the previous build are lost by github, this is a useful information that is duly recorded by the CPython buildbot system. FWIW this change in the last commit in the test case was motivated because the previous Appveyor (discarded) test reported that the ResourceWarning could not be found in stderr.

I will wait for a stabilisation of the github tooling before going any further with this (not important) PR. In my last PR, PR 132, I had to run multiple noop 'Merge branch 'master' into bpo-20210-master' to trigger a new build because those builds always failed with some '60 minutes time exceeded' cryptic message.

I suppose this makes me one of those people that are definitely not "real men" :-)

[xdegaye]

It is sad that the results of the previous build are lost by github.

Actually this can be found by clicking on the icon (either a green tick mark or a red cross) on the right of the commit message in this page.

[xdegaye]

This is the conflict that is being reported by github !

from test.support import (TESTFN, captured_stderr, check_impl_detail,
                          check_warnings, cpython_only, gc_collect, run_unittest,
<<<<<<< bpo-30697
                          no_tracing, unlink, import_module, script_helper,
                          SuppressCrashReport)
=======
                          no_tracing, unlink, import_module, script_helper)
>>>>>>> master

class NaiveException(Exception):
    def __init__(self, x):
        self.x = x

[miss-islington]

Thanks @xdegaye for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

[miss-islington]

Sorry, @xdegaye, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 56d1f5ca32892c7643eb8cee49c40c1644f1abfe 3.6

[bedevere-bot]

GH-4135 is a backport of this pull request to the 3.6 branch.