Listen for added DBus interfaces instead of registered services
ClosedPublic

Authored by fvogt on Oct 27 2018, 10:56 PM.

Details

Summary

When the service is registered, the interfaces might not be available yet.
So wait for those instead.

BUG: 400359

This was RFC because:

  • I'm not sure whether it's guaranteed that InterfacesAdded is emitted on service registration as well or both connections are necessary
  • I don't know how it's possible to test this as org.kde.fakenetwork does not support the org.freedesktop.ObjectManager API
Test Plan

Restarting NM while kded5 keeps running caused "nmcli up" to fail because of
missing secrets.
With this patch, it works successfully.

Diff Detail

Repository
R282 NetworkManagerQt
Branch
ifadded
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4254
Build 4272: arc lint + arc unit
fvogt created this revision.Oct 27 2018, 10:56 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 27 2018, 10:56 PM
fvogt requested review of this revision.Oct 27 2018, 10:56 PM
fvogt updated this revision to Diff 44334.Oct 27 2018, 11:34 PM

Fix bad merge

This is RFC because:

I'm not sure whether it's guaranteed that InterfacesAdded is emitted on service registration as well or both connections are necessary

I'm not sure about this either. I'll build it locally and try to use it for a while.

I don't know how it's possible to test this as org.kde.fakenetwork does not support the org.freedesktop.ObjectManager API

The fakenetwork stuff supports or simulates only a small portion of NM dbus, we don't currently test secret agent at all.

Is this supposed to fix also the issue where when you log into a Plasma session, you first get a notification that it failed to activate a connection and then it gets activated successfuly?

fvogt added a comment.Nov 5 2018, 6:45 PM

Is this supposed to fix also the issue where when you log into a Plasma session, you first get a notification that it failed to activate a connection and then it gets activated successfuly?

I only saw that once and was unable to reproduce it, but someone else reported it downstream and posted an NM trace log: https://bugzilla.suse.com/attachment.cgi?id=777407

A more or less educated guess:

  • User logs in
  • NM tries autologin, but no secrets available
  • NM tries WPS as fallback
  • Agent registers
  • WPS returns without success -> notification shown
  • As the agent registered, it queued another autoactivate event

If that's true though it's a bug in NM and this won't fix it.

jgrulich accepted this revision.Nov 6 2018, 7:12 PM

Seems to work, let's push this for now and we will see if there is no regression.

This revision is now accepted and ready to land.Nov 6 2018, 7:12 PM
fvogt retitled this revision from [RFC] Listen for added DBus interfaces instead of registered services to Listen for added DBus interfaces instead of registered services.Nov 6 2018, 7:17 PM
fvogt edited the summary of this revision. (Show Details)
This revision was automatically updated to reflect the committed changes.