search

codypiersall / pynng

Python bindings for Nanomsg Next Generation.
https://pynng.readthedocs.io
MIT License
267 stars 58 forks source link

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

Open broglep-work opened 8 months ago

broglep-work commented 8 months ago

fixes #108

codypiersall commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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.

broglep-work commented 4 months ago

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