GeezerVison

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Is there a way to mark this PR as "don't merge" for now? This is the same PR as before, so it has the same issue. The Azure CI job seems not to be running, which is what exhibited the failure before.

test_unittest still crash on this PR. I tested on Linux (Fedora 32).

Yes, this is the same failure as before. Everything passes locally on my Mac, so it will take me more time to look into this.

Is there a way to mark this PR as "don't merge" for now?

Hum, I wasn't sure, so I did the 3 tricks that I know:

• Add "do no merge label"
• Add "[WIP]" in the PR title
• Convert the PR to a draft

It seems like only the conversion to a draft technically prevents someone to merge the PR my mistake.

Looks like that will cover it. ;)

Okay, I believe my latest changes fix the buildbot failures from before.

Also, for future reference, below is a script I came up with that isolates the behavior difference between Mac and Fedora, with my prior version of the PR:

import re
import sys


def f():
    exc, exc_value, tb = sys.exc_info()
    print(f'CLEARING FRAME: {tb.tb_frame!r}')
    tb.tb_frame.clear()

def g():
    # Uncommenting the following line caused the tb_frame.clear() line
    # above to exhibit the following platform-specific behavior:
    # 1) On Mac, this is logged to stderr
    #  > TypeError: print_exception(): Exception expected for value,
    #    NoneType found
    # 2) On Fedora 32, the following error happens:
    #  > Fatal Python error: Segmentation fault
    data = re.compile('xxx')

    try:
        yield
    except Exception:
        f()

gen = g()
gen.send(None)
gen.throw(ValueError)

Maybe this suggests an issue elsewhere in Python's code base.

Should this PR also get a What's New?

I have made the requested changes; please review again.

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

Thank you.

Regarding an assertion, the thing is that it looks like _PyErr_ChainExceptions() is designed to accept a NULL value. So maybe in the future it can be made to work. (That's why I added an XXX comment.) So I wouldn't want to add an assertion making it seem like it always has to be like that. It could just something about how things are set up right now that was causing the crash.

Would you still like me to add an assertion and/or perhaps a code comment explaining this possibility?

@cjerdonek: Please replace # with GH- in the commit message next time. Thanks!

For future reference, I was able to remove the NULL check in a later PR (#19877): 21893fb