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

Further cleanup on shutdown #419

Merged
merged 2 commits into from
Aug 7, 2019
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Aug 7, 2019

  • restore sys.stdout, etc. prior to closing forwarded io streams.
    This prevents errors if print, excepthook, etc. are called during shutdown, which were happening because we didn't...
  • cleanup heartbeat thread in the right order:
    • terminate context from main thread (context.term is the only threadsafe zmq method)
    • catch resulting ETERM in thread and close socket

cf #412 and jupyter/nbconvert#1052

avoids attempting to use io sockets after they are used,
e.g. by sys.excepthook
- signal exit by calling threadsafe context.term from the main thread
- catch resulting ETERM in heartbeat thread and close the socket cleanly
- suppress ENOTSOCK caused by closing the socket elsewhere (shouldn't be done, but better to catch it than hang)
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 merged commit ca3077e into ipython:master Aug 7, 2019
@minrk minrk deleted the cleanup-close branch August 7, 2019 13:10
@MSeal
Copy link
Contributor

MSeal commented Aug 7, 2019

Thanks you @minrk !

@MSeal MSeal mentioned this pull request Aug 7, 2019
@willingc
Copy link
Member

willingc commented Aug 7, 2019

Way to go @minrk. Thanks! And @blink1073 for the review too.

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.

4 participants