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

Periodic wireless network scanning is ruining network performance #306

Closed
Coornail opened this issue Feb 14, 2017 · 33 comments
Closed

Periodic wireless network scanning is ruining network performance #306

Coornail opened this issue Feb 14, 2017 · 33 comments
Assignees
Milestone

Comments

@Coornail
Copy link

Tl;dr

  • KeepassXC periodically scans wireless networks (for location information?)
  • Scanning wireless networks causes the network card to switch to monitor mode
  • In monitor mode it drops all packets. This lasts about 20-30ms, which is enough to ruin network performance.

Context

I noticed that my wireless performance is not as it should be, started investigating. Jumped through several hoops and I noticed that there is a periodic packet drop in about every second.

Looked through the wireless logs, and found several entries like:

Tue Feb 14 18:44:27.228 AutoJoin: <airportd[74]> Successful cache-assisted scan request for KeePassXC with channels {(
...
Tue Feb 14 18:44:27.228 )} took 0.8821 seconds, returned 0 results
Tue Feb 14 18:44:27.228 Scan: <airportd[74]> Cache-assisted scan request for KeePassXC on channel 112 does not require a live scan
Tue Feb 14 18:44:27.228 Info: <Wi-Fi Menu Extra[321]> scan cache updated

I quit KeepassXC, and bam, my network performance is back to normal.

Here are some graphs, I used iperf on my router, first without running KeepassXC, later with KeepassXC:
screen shot 2017-02-14 at 6 45 20 pm

Expected Behavior

KeepassXC should not look for location information at all.

Current Behavior

KeepassXC ruins other programs network performance without even opening a network socket.

Your Environment

  • MacBookAir6,2
  • MacOs Sierra 10.12.3 (16D32)
@phoerious
Copy link
Member

I'm not quite sure what's happening there. Do you have KeePassHTTP enabled?

@Coornail
Copy link
Author

No, I don't have it enabled.

@phoerious
Copy link
Member

So I assume

lsof -i -a -p <PID-OF-KEEPASSXC>

does not yield any output? Then there is really no reason why KeePassXC should do anything with the network. Now I need some MacBook with a wireless card to reproduce. 😖

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Feb 14, 2017

This doesn't even make sense to me anymore...
Where have you downloaded the .dmg from? Have you checked the SHA256 or Signature from our website?

@phoerious
Copy link
Member

phoerious commented Feb 14, 2017

I found something: https://muut.com/i/odrive/general:wifi-scan-requests-by-odriv

Quote:

Hi, Thanks for pinging us on this. We are not intentionally scanning Wifi networks, so this is definitely odd. We are looking into it now.
...
It looks like there was some generic net code in a lower-level UI framework library we used that was doing this as part of a port initialization routine. Fortunately, that whole framework has been removed in our current development builds, so this behavior will go away on the next release. Thanks for the report!

That is about what I was expecting too. Apple seems to like these kind of quirks in their broken SDKs. Or maybe this is a Qt bug on OS X?
I wish they told us what exactly caused the scans.

@Coornail
Copy link
Author

@phoerious lsof gives nothing

@TheZ3ro I installed keepassxc via brew cask: https://github.com/caskroom/homebrew-cask/blob/master/Casks/keepassxc.rb
It does check the sha256 digest, as I see, I did not check it by hand.

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Feb 14, 2017

@Coornail ok the cask is fine

@phoerious they are talking about "port initialization routine". It can't be libmicrohttpd ? He is using Sierra so the HTTP plugin should be enabled. Can you check in the about dialog?? @Coornail

But they also say that is "some generic net code in a lower-level UI framework library"

@phoerious
Copy link
Member

phoerious commented Feb 14, 2017

@Coornail The Homebrew cask contains libmicrohttpd. Could you please download the DMG without KeePassHTTP support from our website and check if the issue persists?

@phoerious
Copy link
Member

@TheZ3ro Yes, libmicrohttpd is what I suspect.

@Coornail
Copy link
Author

@phoerious I downloaded the non-libmicrohttpd build from the website (https://github.com/keepassxreboot/keepassxc/releases/download/2.1.1/KeePassXC-2.1.1.dmg).

I could reproduce the periodic package-loss with this one as well.

@phoerious
Copy link
Member

This is getting very mysterious and I have no way of reproducing it. Tricky situation. Any other Mac users here who experience the same problem?

@Coornail
Copy link
Author

One thing that might help: this issue doesn't happen with the original KeepassX program.

@phoerious
Copy link
Member

phoerious commented Feb 14, 2017

That doesn't really help. Too much has changed and I can't bisect the full history and compile you a hundred DMGs to test.
But what you maybe could do is building the develop branch of KeePassX on Qt 5.8. That would at least clarify whether it's Qt's fault or something else. You can basically follow the build instructions from our Wiki, but you don't need libmicrohttpd:

https://github.com/keepassxreboot/keepassxc/wiki/Set-up-Build-Environment-on-OS-X

https://github.com/keepassxreboot/keepassxc/wiki/Building-KeePassXC#os-x

@Coornail
Copy link
Author

@phoerious
Copy link
Member

phoerious commented Feb 14, 2017

That, on the other hand, is indeed interesting.
Could you please open the file src/gui/EditWidgetIcons.cpp and replace line 43

    , m_networkAccessMngr(new QNetworkAccessManager(this))

with

    , m_networkAccessMngr(nullptr)

and recompile?

Don't try to download a favicon after this, it will crash the application. But I'd be interested in whether you are still experiencing those problems.

@phoerious
Copy link
Member

phoerious commented Feb 14, 2017

Great find! The call trace really helped. Apparently, this is a Qt bug which has only been fixed on Windows, if at all.
https://bugreports.qt.io/browse/QTBUG-48491
https://bugreports.qt.io/browse/QTBUG-40332

If you could now confirm that not instantiating QNetworkAccessManager gets rid of the problem, we found the root cause.

@phoerious phoerious added this to the v2.1.2 milestone Feb 14, 2017
phoerious added a commit that referenced this issue Feb 15, 2017
…olves #306

Also replace some value calls with const reference calls to prevent unnecessary copying of QObjects
@phoerious
Copy link
Member

Assuming QNetworkAccessManager is indeed the issue, I prepared a patch: 2ccf33d
Could you test if that solves your problems?

@varjolintu
Copy link
Member

I was also facing this issue (macOS Sierra 10.12.3 with Qt 5.8.0.1) but the patch fixed it. Thanks!

@Coornail
Copy link
Author

I built 2ccf33d and it indeed fixes the network problem!

Thanks for the fast reply, great job!

@phoerious
Copy link
Member

phoerious commented Feb 15, 2017 via email

@varjolintu
Copy link
Member

Downloading a favicon breaks it again.

@phoerious
Copy link
Member

Permanently? Oh jeez. Now it's getting complicated.

@varjolintu
Copy link
Member

Yes. But before the patch the scans happened every 3 seconds (sometimes even 10). After downloading the favicon the scans are between 10 seconds. Sometimes even 20. So they are not so frequent.

@phoerious
Copy link
Member

That's not good enough. I suppose there is some background logic going on that isn't properly stopped when the QNetworkAccessManager ist deleted. Give me some time to prepare a new fix.

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Feb 15, 2017

This is the patch they have done for WindowsXP https://codereview.qt-project.org/#/c/114816/

@phoerious
Copy link
Member

Since we can't change the Qt source code, we need to find a higher-level solution for this. I'll experiment with a little more after lunch.

@phoerious phoerious self-assigned this Feb 15, 2017
@droidmonkey
Copy link
Member

Wow this is terrible on Qt's part. They need to groom their netowrk code badly.

@phoerious
Copy link
Member

phoerious commented Feb 15, 2017

I completely re-implemented everything with a small third-party network library. Please test the hotfix/306-wifi-network-polling (head revision: f230bae) branch (reset any changes if you checked out the previous branch).

@varjolintu
Copy link
Member

After the hotfix I have not seen a single airportd query in the Console. Seems to work. Can anyone else confirm this?

@phoerious
Copy link
Member

Also after downloading favicons?

@varjolintu
Copy link
Member

Yes :)

@phoerious
Copy link
Member

That's great to hear. I'll make the patch ready for review.

@Coornail
Copy link
Author

Tested the hotfix/306-wifi-network-polling branch, works as it should.
Tried downloading a favicon as well.

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants