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.

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

fix: don't call nng_fini() by default #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

broglep-work
Copy link

fixes #108

@codypiersall
Copy link
Owner

Thanks for the PR! I'm wondering how this is related to #55. I am worried that if we don't call nng_fini by default, we will cause the deadlocks that #55 fixed to come back. Seems like the situation may just be less than ideal, all around.

Making the atexit behavior configurable like you've done here may be a nice stop-gap.

@broglep-work
Copy link
Author

We did not observe deadlocks with this fix, we have it in use for quite a while now.
Based on the bug report in #55 I'm not sure how to test if this fix here will bring that issue back

@codypiersall
Copy link
Owner

CI is broken, totally unrelated to this merge request. I will merge this pull request either after the windows CI starts working again, or after I get tired of trying to fix CI, whichever comes first.

It's very likely that I will just merge this after giving up on CI—the only thing that is stopping me from merging right now is that I don't want the "Build passing" badge in the README to turn into a "Build failed" badge. It's not the best reason, but it is mine.

mlasch pushed a commit to husqvarnagroup/pynng that referenced this pull request Apr 11, 2024
mlasch pushed a commit to husqvarnagroup/pynng that referenced this pull request Apr 11, 2024
@broglep-work
Copy link
Author

@codypiersall as the build pipeline seems working now, I have rebased the PR. Hope the PR can get merged now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NNG Panic on Python shutdown
2 participants