Skip to content

Qt Ctrl-C broken on windows #20932

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.

Sign up for GitHub

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

Jump to bottom
Closed
tacaswell opened this issue Aug 28, 2021 · 9 comments
Closed

Qt Ctrl-C broken on windows #20932

tacaswell opened this issue Aug 28, 2021 · 9 comments
Labels
GUI: Qt OS: Microsoft Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone
v3.5.0

Comments

@tacaswell
Copy link
Member

@anntzer @tacaswell
Hi! I have just tested the master branch on Windows and found that there is indeed a problem with the QSocketNotifier (it appeared here: #13306 (comment)). Previously, on that Windows machine, I was running my old code with QAbstractSocket which was fine, and did not test the newest version with QSocketNotifier there (my fault, sorry). I have tested on Linux, and there is no such problem there.

The code to reproduce on Windows (in Jupyter):

%matplotlib qt
from matplotlib import pyplot as plt
plt.plot([1,2])
plt.pause(.1)
plt.pause(.1)
plt.pause(.1)

The problem seems to be that if socketpair generates a socket with the same fileno value as it generated in the previous pause() call, the QSocketNotifier unexpectedly fires and hangs here:

matplotlib/lib/matplotlib/backends/qt_compat.py

Line 232 in b600026

sn.activated.connect(lambda *args: rsock.recv(1))

because nothing was actually written to the wakeup_fd. I don't understand how that can be.

Workaround fix:

rsock.set_blocking(False)

@sn.activated.connect
def on_signal(*args):
    try:
        signal = rsock.recv(1)  # clear the socket to re-arm the notifier
    except BlockingIOError:
        pass # false-trigger, nothing on rsock

What should we do with this? Is there a better solution?

Originally posted by @vdrhtc in #13306 (comment)
@tacaswell tacaswell added this to the v3.5.0 milestone Aug 28, 2021
@tacaswell tacaswell added GUI: Qt OS: Microsoft Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Aug 28, 2021
@QuLogic
Copy link
Member

QuLogic commented Aug 30, 2021

Is it the current QSocketNotifier that fires, or the previous one? Are we just missing some cleanup?

@vdrhtc
Copy link
Contributor

vdrhtc commented Aug 31, 2021 •
edited
Loading

@QuLogic
image

Here how it looks like, I don't think it is the same QSocketNotifier, it is a different object every time. But false-trigger only happens if the fileno is re-used (1092 in this case, for 1028 nothing happens). So maybe it is necessary to clear the rsock somehow.

@QuLogic
Copy link
Member

QuLogic commented Sep 1, 2021

Does something like this work?

import select
import socket


for i in range(10):
    wsock, rsock = socket.socketpair()
    wsock.setblocking(False)
    rfd = rsock.fileno()
    wfd = wsock.fileno()
    print('rsock:', rfd, 'wsock:', wfd)
    ready = select.select([rfd], [], [], 2)
    print('ready:', ready)
    wsock.close()
    rsock.close()

It should print out that no sockets are ready. Windows support was only added in 3.5, but maybe there's a bug?

Maybe we should call socket.shutdown on the sockets to force them to be disabled, even though we do close them:

Shut down one or both halves of the connection. If how is SHUT_RD, further receives are disallowed. If how is SHUT_WR, further sends are disallowed. If how is SHUT_RDWR, further sends and receives are disallowed.

@QuLogic
Copy link
Member

QuLogic commented Sep 1, 2021

Also, does reusing the existing socket pair work?

wsock, rsock = socket.socketpair()
wsock.setblocking(False)
sn = QtCore.QSocketNotifier(
    rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read
)
sn.activated.connect(lambda *args: print('activated 1'))
del sn
sn = QtCore.QSocketNotifier(
    rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read
)
sn.activated.connect(lambda *args: print('activated 2'))

@vdrhtc
Copy link
Contributor

vdrhtc commented Sep 1, 2021

Nothing unexpected in the first test despite occasional re-uses:
image

Second test also works:
image

If I remove the del sn line, then there is a warning about multiple notifiers for the same socket in the terminal, and no activation happens on both notifiers.

Calling

wsock.shutdown(socket.SHUT_RDWR)
rsock.shutdown(socket.SHUT_RDWR)
wsock.close()
rsock.close()

does not help.

@QuLogic
Copy link
Member

QuLogic commented Sep 1, 2021

Actually, for that second test, maybe we should run the event loop in between?

wsock, rsock = socket.socketpair()
wsock.setblocking(False)

sn = QtCore.QSocketNotifier(
    rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read
)
sn.activated.connect(lambda *args: print('activated 1'))
# 5 seconds of event processing should be enough?
event_loop = QtCore.QEventLoop()
timer = QtCore.QTimer.singleShot(5000, event_loop.quit)
qt_compat._exec(event_loop)

del sn

sn = QtCore.QSocketNotifier(
    rsock.fileno(), _enum('QtCore.QSocketNotifier.Type').Read
)
sn.activated.connect(lambda *args: print('activated 2'))
# 5 seconds of event processing should be enough?
event_loop = QtCore.QEventLoop()
timer = QtCore.QTimer.singleShot(5000, event_loop.quit)
qt_compat._exec(event_loop)

@vdrhtc
Copy link
Contributor

vdrhtc commented Sep 16, 2021

@QuLogic For some reason, the event loop does not exit after 5s and then the program hangs...

I've obtained the previous results by running %matplotlib qt first (it probably starts the event loop correctly by itself)

@timhoffm
Copy link
Member

timhoffm commented Oct 1, 2021

Is this fixed by #20907?

@QuLogic
Copy link
Member

QuLogic commented Oct 1, 2021

It should be.

@QuLogic QuLogic closed this as completed Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GUI: Qt OS: Microsoft Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.5.0
Development

No branches or pull requests
4 participants
@tacaswell @QuLogic @timhoffm @vdrhtc