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

Fix QSocketNotifier in the Qt event loop not being disabled for the control channel #525

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Jul 15, 2020

This was generating a flush of warnings in qtconsole and Spyder.

Fixes jupyter/qtconsole#332
Fixes spyder-ide/spyder#13275

…ontrol channel

This was generating a constant warning in qtconsole and Spyder.
@blink1073
Copy link
Contributor

Hi @ccordoba12! Is this good to go?

@ccordoba12
Copy link
Member Author

Yes, it is. I was waiting for confirmation from others, and @dalthviz (one of our core devs) and some of our users reported it solved the problem for them.

I removed the reference to Vispy because I didn't hear from @hmaarrfk nor @djhoese, but I hope this will solve it for them too.

# notifier in order to connect a new one in the next
# execution. This applies to the control channel.
notifier.setEnabled(False)
kernel.app.quit()
Copy link

Choose a reason for hiding this comment

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

Doesn't putting this in an else make the entire if/else block unnecessary (both True/False cases run the exact same code)?

Copy link
Member Author

@ccordoba12 ccordoba12 Jul 16, 2020

Choose a reason for hiding this comment

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

I thought about that too, but the if also flushes the shell channel. So perhaps I should rewrite this to

stream.flush(limit=1)
notifier.setEnabled(False)
kernel.app.quit()

?

Copy link

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

Is this meant to stop the repeated issuing of the QSocketNotifier warning or does it stop it from appearing completely?

@ccordoba12
Copy link
Member Author

ccordoba12 commented Jul 16, 2020

Is this meant to stop the repeated issuing of the QSocketNotifier warning or does it stop it from appearing completely?

It stops it completely because now we disable the previous notifier, so that a new one can be activated for the same Zmq socket.

The problem is that, according to the Qt docs:

[I]t is not possible to install two socket notifiers of the same type (Read, Write, Exception) on the same socket.

So, since flushing the control channel didn't return anything (because it's an extra channel that for now it's only used by the JupyterLab debugger, I think), we were not deactivating the previous notifier. And that triggered the warning.

@blink1073 blink1073 merged commit 8123259 into ipython:master Jul 17, 2020
@blink1073
Copy link
Contributor

Released as 5.3.3.

@ccordoba12 ccordoba12 deleted the fix-qsocketnotifier-warning branch July 17, 2020 22:49
@ccordoba12
Copy link
Member Author

Thanks for your help @blink1073! Do you want me to push another PR to improve the code according to @djhoese's suggestions?

I had them prepared but I was busy with other things, sorry.

@ccordoba12
Copy link
Member Author

Pinging @anaconda-pkg-build about this one. It'd be really great if you could include this version in the upcoming Anaconda release. Thanks!

@jacky4eyes
Copy link

Hi,

I'm not sure whether this is the right place for reporting issues, but my qt in jupyter doesn't work any more after I upgraded ipykernel today from 5.3.0 to 5.3.3. Downgrading it back to 5.3.0 solves my problem, so I believe there's probably some incompatibility in this 5.3.3 version.

Here is what I got:

Test this code in jupyter notebook:

%matplotlib qt
import matplotlib.pyplot as plt
x = [1,2,3]
y = [5,4,5]
plt.plot(x,y)

Before upgrade to ipykernel==5.3.3
image

After:
image

@pchristian4481
Copy link

Installed ipykernel==5.3.3 and reinstalled spyder 4.1.4 and that seemed to fix the socket issue, so far.

Thanks

@blink1073
Copy link
Contributor

Hi @jacky4eyes, can you please open a new issue? I'm also curious if installing 5.3.2 fixes the problem, which would help narrow it down.

@Wuhuuu2
Copy link

Wuhuuu2 commented May 7, 2021

Hi all,
the last post is quite some time ago, but I have to say that I still get this error message.
All i need to create the error:
"QSocketNotifier: Multiple socket notifiers for same socket and type Read"

import matplotlib.pyplot as plt
fig = plt.figure()
plt.show()

However there is an interesting thing about it:
If I copy this code into a new console, the message does not show up.
If I run this code from inside a file, like test.py it does show up (I excecute it in a new console).
Now, if I execute my test.py file, but either comment everything or prevent execution with import sys and sys.exit(), just a new console opens. Now running the uncommented code from test.py inside this previously new created console, the error does not show up.
I feel thats interestingly strange. Maybe it helps you find the source again.

Name Version Build Channel

ipython                   7.22.0           py39hd4e2768_0
ipython_genutils          0.2.0            pyhd3eb1b0_1
python                    3.9.4            h6244533_0
python-dateutil           2.8.1            pyhd3eb1b0_0
python-jsonrpc-server     0.4.0            py_0
python-language-server    0.36.2           pyhd3eb1b0_0
python-slugify            5.0.0            pyhd3eb1b0_0
python_abi                3.9              1_cp39    conda-forge
ipykernel                 5.3.4            py39h7b7c402_0
matplotlib                3.3.4            py39haa95532_0
matplotlib-base           3.3.4            py39h49ac443_0

base environment:
anaconda                  2019.10                  py37_0
anaconda-client           1.7.2                    py37_0
anaconda-navigator        2.0.1                    py37_0
anaconda-project          0.8.3                      py_0
conda                     4.10.1           py37haa95532_1
conda-build               3.18.9                   py37_3
conda-content-trust       0.1.1              pyhd3eb1b0_0
conda-env                 2.6.0                         1
conda-package-handling    1.7.3            py37h8cc25b3_1
conda-repo-cli            1.0.4              pyhd3eb1b0_0
conda-token               0.3.0              pyhd3eb1b0_0
conda-verify              3.4.2                      py_1
msys2-conda-epoch         20160418                      1

I am not sure anymore, but it could be the case that this error showed up after i got a promt (by anaconda) to update conda navigator. (And did update it)

Solution

As found under the commit tap, adding the else loop part also helps in this case.
But I thought it´s supposed to be corrected already, but maybe not (anymore)

@hmaarrfk
Copy link

hmaarrfk commented May 7, 2021

I have alot of questions about your setup.

It seems you are maybe mixing anaconda base with conda-forge stuff and are likely using old versions of certain packages.

I'm not too sure, but it would help if we could get more organized details about your setup.

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 this pull request may close these issues.

QSocketNotifier: Multiple socket notifiers for same socket and type Read QSocketNotifier warning in terminal
7 participants