psi-plus / main

Main repository with patches and required resources
https://psi-plus.com/
GNU Lesser General Public License v2.1
69 stars 20 forks source link

psi+ does not reconnect on resume on wifi (regression) #736

Closed jirislaby closed 5 years ago

jirislaby commented 5 years ago

I am using psi+ for ages. Lately, when resuming from suspend, psi+ won't reconnect when I am on wifi. On cable, everything is OK. I am on linux, openSUSE Tumbleweed.

Psi+ v1.3.396 (2018-07-05, Psi:5ed84161, Psi+:ff32f79, webkit) libQt5Core5-5.11.1-3.3.x86_64

systemctl status network
● NetworkManager.service - Network Manager
   Loaded: loaded (/usr/lib/systemd/system/NetworkManager.service

The thing is that my father, using the same version, but on Win 10, reports the same (cable not tested).

We both have all the options in account settings checked (auto reconnect on run, sleep, after disconnect, …).

Has anything changed in this area?

tehnick commented 5 years ago

It is a pity that you have faced with such bug. IIRC, we have used own crutches for this feature in the past, but now the standard Qt functions for notifying on network connection status are used, so it should work better... @Ri0n may say more on this topic.

Also we have added Stream Management feature in Psi+ and in very rare cases it may work not as expected. Try to disable "Enable Stream Management if possible" option in connection settings and check if it affects anything.

Ri0n commented 5 years ago

Yes. Correct. there is a combination of Qt notifications and some custom suspend monitoring. I recently rewrote this code to reconnect as soon as possible after resume. But yes, in some very rare cases it fails to reconnect. I was unable to reproduce it in debugger. So any help will be very appreciated.

jirislaby commented 5 years ago

@Ri0n help in what sense? Should I collect some info like adding breakpoints or debug prints somewhere?

@tehnick I have just disabled Stream Management and see what happens…

Ri0n commented 5 years ago

@jirislaby, that would be perfect. I can tell more about the code later.

Ri0n commented 5 years ago

there is systemwatch_unix.cpp file responsible for sleep/resume. So first make sure you compile with dbus on your SUSE.

Then you can go to psicon.cpp and check when and how doSleep() and doWakeup() are called.

PsiCon::networkSessionOpened() also needs some debug prints.

Regarding breakpoints. It's really hard to debug sleep/resume with them. So better just prints.

jirislaby commented 5 years ago

@Ri0n I instrumented those functions and the log is: psi-log.txt

grep XXX of the log:

[20181004 9:46:58] XXX void PsiCon::networkSessionOpened() false (unknown:0, unknown)
[20181004 9:46:59] XXX UnixSystemWatch::UnixSystemWatch() (unknown:0, unknown)
[20181004 9:47:13] XXX void UnixSystemWatch::prepareForSleep(bool) true (unknown:0, unknown)
[20181004 9:47:13] XXX void PsiCon::doSleep() (unknown:0, unknown)
[20181004 9:47:34] XXX void UnixSystemWatch::prepareForSleep(bool) false (unknown:0, unknown)
[20181004 9:47:34] XXX void PsiCon::doWakeup() (unknown:0, unknown)
[20181004 9:47:37] XXX void PsiCon::networkSessionOpened() true (unknown:0, unknown)
[20181004 9:48:45] XXX void UnixSystemWatch::prepareForSleep(bool) true (unknown:0, unknown)
[20181004 9:48:45] XXX void PsiCon::doSleep() (unknown:0, unknown)
[20181004 9:48:56] XXX void UnixSystemWatch::prepareForSleep(bool) false (unknown:0, unknown)
[20181004 9:48:56] XXX void PsiCon::doWakeup() (unknown:0, unknown)

20181004 9:47:34 is resume with wired lan -> psi+ connected 20181004 9:48:56 is resume with wireless lan -> psi+ did not connect

And the difference is clear: networkSessionOpened is and is not called, respectively.

jirislaby commented 5 years ago

The whole problem is only when I suspend on LAN and resume on WLAN (or vice versa). Or more generally on a different QNetworkConfiguration. I added few more signals handling:

[20181004 10:55:16] XXX void PsiCon::networkSessionOpened() pending false iface "Auto eth" (unknown:0, unknown)
[20181004 10:55:17] XXX UnixSystemWatch::UnixSystemWatch() (unknown:0, unknown)
[20181004 10:55:22] XXX void UnixSystemWatch::prepareForSleep(bool) true (unknown:0, unknown)
[20181004 10:55:22] XXX void PsiCon::doSleep() (unknown:0, unknown)
[20181004 10:55:29] XXX void UnixSystemWatch::prepareForSleep(bool) false (unknown:0, unknown)
[20181004 10:55:29] XXX void PsiCon::doWakeup() (unknown:0, unknown)
[20181004 10:55:31] XXX void PsiCon::networkSessionClosed() default "tun0" (unknown:0, unknown)
[20181004 10:55:31] XXX void PsiCon::networkSessionOpened() pending true iface "tun0" (unknown:0, unknown)
[20181004 10:55:31] XXX void PsiCon::networkSessionChanged(QNetworkSession::State) 5 (unknown:0, unknown)

The above is with this hack (which is completely and utterly wrong):

void PsiCon::networkSessionClosed()
{
    qDebug() << "XXX" << __PRETTY_FUNCTION__ << "default" <<
            d->netConfMng.defaultConfiguration().name();
//    delete d->netSession;
    d->netSession = new QNetworkSession(d->netConfMng.defaultConfiguration(), this);
    connect(d->netSession, &QNetworkSession::opened, this, &PsiCon::networkSessionOpened);
    connect(d->netSession, &QNetworkSession::closed, this, &PsiCon::networkSessionClosed);
    connect(d->netSession, &QNetworkSession::stateChanged, this, &PsiCon::networkSessionChanged);
    d->netSession->open();
}

It indeed works. So you have to introduce roaming to the code or something. Because suspending with one wireless access point and resuming with another one is broken now too (which is what my father is likely seeing).

Ri0n commented 5 years ago

Thanks for the investigation! If you can make a PR with a workaround (and corresponding comments in the code) that would be great. We can redesign it later. I thought about roaming but had no spare time to look at it more closely.