Skip to content

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

Merged
merged 1 commit into from
Jul 1, 2025

Conversation

chrisjbillington
Copy link
Contributor

@chrisjbillington chrisjbillington commented Jun 24, 2025

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

matplotlib.use('QtAgg') 
matplotlib.use('Qt5Agg') 

Previously, running the following with python -i script.py:

import matplotlib.pyplot as plt
plt.plot([1, 2, 3], [4, 5, 6])
plt.show()

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:

import matplotlib.pyplot as plt
plt.plot([1, 2, 3],[5, 4, 6])
plt.pause(10)

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 single KeyBoardInterrupt traceback as expected.

PR checklist

I'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.

Copy link

@github-actions github-actions bot left a 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.

@chrisjbillington
Copy link
Contributor Author

Though I haven't seen official mention of the need to call .deleteLater(), I certainly have seen before that Python reference counts reaching zero isn't enough to clean up all state associated with QObjects in PyQt.

Here's are some mentions of this on the web:

It does seem, from running gc.get_referrers(), that there aren't any other Python references to this QSocketNotifier, so it does seem to be living on exclusively in the C++ layer unless explicitly deleted.

@chrisjbillington
Copy link
Contributor Author

Oh I see. _allow_interrupt() is backend-agnostic, so Qt things don't belong there, hence the tests failing on macos. This will have to be done differently.

@chrisjbillington
Copy link
Contributor Author

chrisjbillington commented Jun 24, 2025

Changed to put the cleanup in the backend-specific code by connecting the event loop's aboutToQuit signal to the socket notifier's deleteLater() method edit: (that doesn't work for QEventLoop) by explicitly calling sn.deleteLater() from handle_sigint().

I've tested with PyQt6, PySide6, PyQt5, and PySide2. Looks like this was only ever an issue in PyQt:

            before    after
PyQt6          bad    good  
PySide6       good    good
PyQt5          bad    good
PySide2       good    good

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.

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.
@QuLogic QuLogic added this to the v3.10.4 milestone Jun 30, 2025
@anntzer
Copy link
Contributor

anntzer commented Jul 1, 2025

Thanks for the patch!

@anntzer anntzer merged commit 413ea8f into matplotlib:main Jul 1, 2025
41 checks passed
@github-project-automation github-project-automation bot moved this from Needs review to Waiting for author in First Time Contributors Jul 1, 2025
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jul 1, 2025
@chrisjbillington chrisjbillington deleted the qt-interrupt-cleanup branch July 1, 2025 08:20
QuLogic added a commit that referenced this pull request Jul 1, 2025
…209-on-v3.10.x

Backport PR #30209 on branch v3.10.x (Clean up Qt socket notifier to avoid spurious interrupt handler calls)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for author
Development

Successfully merging this pull request may close these issues.

[Bug]: "Bad file descriptor" raised repeatedly when plt.pause() interrupted in IPython
3 participants