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

Cannot exit ipython with running qcodes.Monitor() #5561

Closed
michaelc12qe opened this issue Nov 28, 2023 · 4 comments · Fixed by #5565
Closed

Cannot exit ipython with running qcodes.Monitor() #5561

michaelc12qe opened this issue Nov 28, 2023 · 4 comments · Fixed by #5565
Labels

Comments

@michaelc12qe
Copy link
Contributor

michaelc12qe commented Nov 28, 2023

Steps to reproduce

iPython version 8.10, qcodes version 0.42, Windows 10 Pro 22H2

  1. open ipython
  2. from qcodes import Monitor
    monitor = Monitor()
  3. exit ipython

It hangs instead of closing iPython. The terminal must be terminated.

@jenshnielsen
Copy link
Collaborator

I can reproduce this. Not sure what is going on but perhaps related to #3814

It would be nice to investigate if changing the monitor thread to be a deamon thread would also resolve this issue

@jenshnielsen
Copy link
Collaborator

Note that until this is resolved it can be mitigated by explicitly calling join on the monitor thread

❯ ipython
Python 3.11.5 | packaged by Anaconda, Inc. | (main, Sep 11 2023, 13:26:23) [MSC v.1916 64 bit (AMD64)]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.18.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from qcodes import Monitor
In [2]: m = Monitor()
In [3]: m.join()
In [4]: exit

~ took 29s
❯

@michaelc12qe
Copy link
Contributor Author

I've tested turning the monitor thread into a daemon thread as you suggested, by passing the keyword argument daemon=True into the Monitor initialization. It appears to resolve this issue. I've also checked that the behaviour of the monitor remains unchanged by add QDAC2 voltage channels to the monitor, and it seems fine.

I don't know if there's any more testing required, but happy to raise a PR for this if you like.

@jenshnielsen
Copy link
Collaborator

jenshnielsen commented Nov 29, 2023

@michaelc12qe In the long term I would like to come up with something slightly cleaner than deamon mode but I agree that in the short term using deamon is the best solution. So please go ahead

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

Successfully merging a pull request may close this issue.

2 participants