Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bugfix: Keep registered types until after Py_Finalize(). Fix #4459 #4486

Merged
merged 2 commits into from
Feb 6, 2023
Merged

bugfix: Keep registered types until after Py_Finalize(). Fix #4459 #4486

merged 2 commits into from
Feb 6, 2023

Conversation

Skylion007
Copy link
Collaborator

@Skylion007 Skylion007 commented Feb 2, 2023 •
edited by rwgk
Loading

Description

NOTE: rolled back with PR #4501

Fix #4459 . Keep types registered until after atexit is called.

TODO / help wanted: unit test exercising the atexit situation.

Suggested changelog entry:

* Fix bug where cpptypes were unregistered before atexit was called.

rwgk
rwgk approved these changes Feb 2, 2023
View reviewed changes
Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK to let the testing slip, in the interest of getting the fix in sooner rather than later, but it would be best to mention that clearly in the PR description, e.g.:

TODO / help wanted: unit test exercising the atexit situation.

include/pybind11/embed.h Outdated
// Local internals contains data managed by the current interpreter, so we must clear them to
// avoid undefined behaviors when initializing another interpreter
// Must be cleared after Py_Finalize() so atexit and other hooks can use registered_types
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Must be cleared only after Py_Finalize() so atexit and other hooks can still use use registered_types

?

Extra "only" and "still", to emphasize the importance.

@Skylion007 Skylion007 changed the title (bugfix): Keep registered types until after Py_Finalize(). Fix #4459 bugfix: Keep registered types until after Py_Finalize(). Fix #4459 Feb 4, 2023
@Skylion007 Skylion007 merged commit b2c1978 into pybind:master Feb 6, 2023
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 6, 2023
rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Feb 8, 2023
Revert "bugfix: Keep registered types until after Py_Finalize(). Fix p…
4efdec8 
…ybind#4459 (pybind#4486)"

This reverts commit b2c1978.

See pybind#4500 for background.
rwgk pushed a commit that referenced this pull request Feb 8, 2023
Revert "bugfix: Keep registered types until after Py_Finalize(). Fix #…
f8713ec 
…4459 (#4486)" (#4501)

This reverts commit b2c1978.

See #4500 for background.
@rwgk rwgk mentioned this pull request Feb 8, 2023
Merged
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Feb 8, 2023
@rwgk rwgk mentioned this pull request Feb 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwgk rwgk rwgk approved these changes

@henryiii henryiii Awaiting requested review from henryiii

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: Python atexit broken with pybind11 version >= 2.9.2
2 participants
@Skylion007 @rwgk