bjoern
bjoern copied to clipboard
jonashaag
Move server socket teardown to an atexit handler
I see! Thanks for the fix!
I wonder what happens if you invoke .run() multiple times in the same Python interpreter. Not sure how atexit behaves. Also the teardown is done only when the interpreter exits, shouldn't we close the socket as soon as possible? Or better even, do both, i.e. have a teardown in a finally block as before this patch and an atexit handler?
We'll also have to check if the issue reported in #146 is even reproducible anymore with the fix in #91 – and then we'll also have to re-check when we make any changes resulting from the discussion in #91
I don't see any commited code referenced in #91, only discussion; am I missing something?
As for calling run() multiple times, that's handled before the atexit register happens (it would throw a RuntimeError).
I just reviewed the atexit docs and realized there's a problem with my fix.
The functions registered via this module are not called when the program is killed by a signal not handled by Python, when a Python fatal internal error is detected, or when os._exit() is called.
Replacing the try/except block that my original commit removed and making the except clause call _close_server_socket() would fix this oversight.
Additionally, I'm not super well read into Python's reference counting implementation, but if it doesn't play well with atexit it may be wise to pass sock as an argument to _close_server_socket() instead of relying on the _default_instance global existing. I'm 99% sure that the implementation as it stands will work fine, as I trust that Python handles this, but that 1% of paranoia is still there.
Thanks for looking that stuff up! Let's wait for the resolution of #91 before we continue here.
Welp, I had my head in the code and missed your most recent comment, so the fixes have been implemented regardless haha.