-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Clean up Qt socket notifier to avoid spurious interrupt handler calls #30209
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
Though I haven't seen official mention of the need to call Here's are some mentions of this on the web: It does seem, from running |
8d2c1c4
to
5685b8d
Compare
Oh I see. |
5685b8d
to
10ab66c
Compare
Changed to put the cleanup in the backend-specific code I've tested with PyQt6, PySide6, PyQt5, and PySide2. Looks like this was only ever an issue in PyQt:
The PySide2 testing was not done in a proper dev environment, but by just editing the same file in a matplotlib 3.10.3 installation installed with pip in a Python3.10 virtual environment - seems like the setup process for a proper dev environment requires at least Python 3.11, but the last Python version supported by PySide2 was 3.10. Testing for the other bindings was done in a dev environment on Linux with Python 3.13.3. |
10ab66c
to
14b531d
Compare
7839dd8
to
098532b
Compare
098532b
to
3068e7e
Compare
a257e36
to
037025b
Compare
Closes matplotlib#29688 Objects without a parent are not necessarily cleaned up in `PyQt5/6` when their reference count reaches zero, and must be explicitly cleaned up with `deleteLater()` This prevents the notifier firing after the signal handling was supposed to have been reset to its previous state. Rather than have both `bakend_bases._allow_interrupt()` and `backend_qt._allow_interrupt_qt()` hold a reference to the notifier, we pass it to the backend-specific `handle_signint()` function for cleanup. Note the approach to cleaning up the notifier with `.deleteLater()` followed by `sendPostedEvents()` is the documented workaround for when immediate deletion is desired: https://doc.qt.io/qt-6/qobject.html#deleteLater This ensures the object is still deleted up even if the same event loop does not run again.
037025b
to
b06d1f4
Compare
Thanks for the patch! |
…urious interrupt handler calls
…209-on-v3.10.x Backport PR #30209 on branch v3.10.x (Clean up Qt socket notifier to avoid spurious interrupt handler calls)
Closes #29688
Objects without a parent are not necessarily cleaned up in PyQt when their reference count reaches zero, and must be explicitly cleaned up with
deleteLater()
This prevents the notifier firing after the signal handling was supposed to have been cleaned up, which can cause spurious errors to be printed if it runs upon further interrupts or at interpreter shutdown.
I've tested this on Linux with Python 3.13, using both of
Previously, running the following with
python -i script.py
:and then pressing ctrl-C in the terminal whilst the figure was shown, would cause a traceback and error message
OSError: [Errno 9] Bad file descriptor
to be printed as the interpreter exited (by e.g. subsequently pressing ctrl-D), and at least for me, cause the controlling terminal to exit for some reason.After this change interpreter shutdown appears clean.
Similarly, as in the original bug report, running
ipython
and then entering in the REPL:and pressing ctrl-C would result in an infinite loop of
OSError: [Errno 9] Bad file descriptor
error messages, and with this change results in a singleKeyBoardInterrupt
traceback as expected.PR checklist
- [] Plotting related features are demonstrated in an exampleN/A- [ ] New Features and API Changes are noted with a directive and release noteN/A- [] Documentation complies with general and docstring guidelinesN/AI've checked the docs build and the tests run - one test fails:
test_font_heuristica[pdf]
but it fails regardless of this change. I have only tested on Linux with Python 3.13.3.