[WIP] Make incoming packet handling multi-threaded
AbandonedPublic

Authored by sredman on Oct 27 2018, 9:58 PM.

Details

Reviewers
None
Group Reviewers
KDE Connect
Summary

Data from multiple sources, such as incoming network packets and dbus calls, can currently cause the entire application thread to block if one call needs to wait on another. This diff is the start of moving LanLinkProvider onto its own thread, such that incoming NetworkPackets can be handled independently for different devices and independently of dbus calls. The question at this point is "Do we even want this"?

Current status is that the daemon crashes if the desktop responds to Android's identity broadcast, but if the desktop initiates the connection it works... Until you try to disconnect!
Packet handling is not actually on its own thread yet

Test Plan
  • Build app
  • Connect devices
  • Use as normal (many crashes ensue)

Diff Detail

Repository
R224 KDE Connect
Branch
multi-thread-link-provider
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 4251
Build 4269: arc lint + arc unit
sredman created this revision.Oct 27 2018, 9:58 PM
Restricted Application added a project: KDE Connect. · View Herald TranscriptOct 27 2018, 9:58 PM
Restricted Application added a subscriber: kdeconnect. · View Herald Transcript
sredman requested review of this revision.Oct 27 2018, 9:58 PM

I would try to avoid the complexity of managing threads, unless there is a clear advantage or issue fixed by this. Has the current implementation given any problems to you?

I would try to avoid the complexity of managing threads, unless there is a clear advantage or issue fixed by this. Has the current implementation given any problems to you?

The issue I was facing is that I needed to be able to block within a dbus call while waiting for the phone to handle a request and return the requested data. Blocking in the dbus call would mean there was no way to handle the incoming packet from the phone

Putting packet handling on its own thread would solve this problem for all plugins, but it would be a huge amount of effort. I have solved it by using a thread in the ConversationDbusInterface. See D16475

If we feel more effort in this direction is worthwhile, this patch is a start. It's more effort than I want to deal with right now, though

sredman abandoned this revision.Nov 7 2018, 3:50 AM

There is no reason to do this. If in the future there is some reason, we may come back to this