[BUG]: Python atexit broken with pybind11 version >= 2.9.2

[chrreisinger]

Required prerequisites

• Make sure you've read the documentation. Your issue may be addressed there.
• Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
• Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

2.10.2

Problem description

We want to upgrade from pybind11 release 2.9.0 to 2.10.2, but we encountered an issue in the Python atexit (https://docs.python.org/3/library/atexit.html) functions, that our custom types are not registered anymore. With delta debugging we found that this PR caused the issue: #3744

In my opinion the

    detail::get_local_internals().registered_types_cpp.clear();    
    detail::get_local_internals().registered_exception_translators.clear();

calls should be after the Py_Finalize function call.

When I change the orders of those function calls, the problem is gone.

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

2.9.1

[Skylion007]

FYI @rwgk

[rwgk]

@chrreisinger could you help by

• creating a PR that moves the .clear() calls after the Py_Finalize() call,
• and ideally adds a unit test for your situation (in tests/test_embed/test_interpreter.cpp)?

The change makes sense to me. My understanding is: the Py_Finalize() invalidates all PyObject* pointers in the local_internals, but looking at the struct local_internals code, it seems to me the .clear() calls don't ever use those pointers. There are a bunch of non-trivial destructors that will run, but none of them will call into the Python C API.

[chrreisinger]

I will give it a try on the weekend.

[rwgk]

I will give it a try on the weekend.

Awesome.

If you create a PR with just the lines moved, that'll be a great start. It will be good to confirm that valgrind does not find any issues. We can work on the test together from there.

[Skylion007]

@chrreisinger Any updates on this?