[Bug 1738838] Re: Bluetooth icon shows "Off" and the only option given below it is "Turn Off"
Bug Watch Updater
1738838 at bugs.launchpad.net
Fri Jan 17 04:18:27 UTC 2020
Launchpad has imported 23 comments from the remote bug at
https://bugzilla.gnome.org/show_bug.cgi?id=782530.
If you reply to an imported comment from within Launchpad, your comment
will be sent to the remote bug automatically. Read more about
Launchpad's inter-bugtracker facilities at
https://help.launchpad.net/InterBugTracking.
------------------------------------------------------------------------
On 2017-05-11T18:58:52+00:00 BenjaminBerg wrote:
Because of the method that the dbus proxies are created there is
currently a race condition. When an adapter is added it can have an
initial "Powered" state of off which is turned on immediately
afterwards. However, the signal connect to get the update only happens
later.
This can be easily reproduced on thinkpads by running "rfkill block 0"
and "rfkill unblock 0" while looking at the gnome-control-center
bluetooth panel.
Attaching a patch that should fix the issue. I hope I didn't totally
misunderstand how the whole DBus proxy stuff is supposed to work.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/0
------------------------------------------------------------------------
On 2017-05-11T19:03:39+00:00 BenjaminBerg wrote:
Created attachment 351663
lib: Rework adapter and device proxy registrations.
This fixes a race condition when a property was updated right after the
object appeared on the bus. Due to the old implementation the property
change would not be registered in that case.
This moves the code to use GDBusObjectManagerClient and using the
provided GObject properties instead of a separate proxy to avoid the
race condition.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/1
------------------------------------------------------------------------
On 2017-05-16T09:15:35+00:00 Bugzilla-x wrote:
Review of attachment 351663:
The patch is unreviewable. Make the hunks bigger and try to split up the
changes if at all possible.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/2
------------------------------------------------------------------------
On 2017-05-16T11:48:04+00:00 BenjaminBerg wrote:
Created attachment 351966
lib: Rework adapter and device proxy registrations.
This fixes a race condition when a property was updated right after the
object appeared on the bus. Due to the old implementation the property
change would not be registered in that case.
This moves the code to use GDBusObjectManagerClient and using the
provided GObject properties instead of a separate proxy to avoid the
race condition.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/3
------------------------------------------------------------------------
On 2017-05-16T11:48:15+00:00 BenjaminBerg wrote:
Created attachment 351967
lib: Always send "default-adapter-powered"
Fix sending of default-adapter-powered in the case that the default
adapter is being disabled.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/4
------------------------------------------------------------------------
On 2017-05-16T11:48:38+00:00 BenjaminBerg wrote:
Created attachment 351968
lib: Force custom icon code override to run
The property setter currently contains some magic to modify the icon for
certain device types. Send a property change notification to force this
code to run after a device has been added.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/5
------------------------------------------------------------------------
On 2017-05-16T11:49:01+00:00 BenjaminBerg wrote:
There, that is all logical changes split out.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/6
------------------------------------------------------------------------
On 2017-05-16T12:27:56+00:00 Bugzilla-x wrote:
I won't have time to review this in the short-term, but a dbusmock
regression test which shows the problem before, and shows it fixed
afterwards would be really useful.
See those in gnome-settings-daemon (though they've been broken for a while for different reasons...):
https://git.gnome.org//browse/gnome-settings-daemon/tree/tests/gsdtestcase.py
https://git.gnome.org//browse/gnome-settings-daemon/tree/plugins/power/test.py
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/7
------------------------------------------------------------------------
On 2017-05-16T17:23:35+00:00 BenjaminBerg wrote:
The patch might be hard to read as it changes all the property accessors, but really it all boils down to the two questions:
* Is it correct to use GDBusObjectManagerClient to keep track of the objects?
* Is it good to use GObject properties on the Adapter1/Device1 proxies (instead of manually driving a DBus properties interface proxy)?
I agree that dbusmock test would be nice in here. But I personally think
adding that testing infrastructure is out of the scope for me here. In
this particular case I would even say that it would be an over specific
test to catch a case of synchronous API abuse.
The race is actually surprisingly obvious. The calls to *_proxy_new_for_bus_sync cause the mainloop to run which means DBus messages are processed too early:
* InterfacesAdded is fired by bluez
* Properties change notification for "Powered" is fired by bluez
* adapter_added runs mainloop for adapter1_proxy_new_for_bus_sync
- at least dbus calls for name resolving happen
* adapter_added runs mainloop for properties_proxy_new_for_bus_sync
- at least dbus calls for name resolving happen
* Properties notification has been lost while running those mainloops
* adapter_added uses cached property values from InterfacesAdded method call
* adapter_added connects to "g-properties-changed" on the proxy
The improvements from bug #701399 made this race condition worse.
With the patch, the code becomes:
* InterfaceAdded is fired on bluez
* Properties change notification for "Powered" is fired by bluez
* GLib creates the Adapter1 proxy object
* glib sends notification about the new object
* connect to Adapter1 happens
* Properties are queried from Adapter1 object through GObject
There simply is no scope for a race condition as connecting/querying
happens in an atomic fashion.
--
Note that there is quite a lot of cleanup potential. In particular
BLUETOOTH_COLUMN_PROPERTIES and creation of the properties proxy can be
removed if this doesn't constitute an API/ABI break.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/8
------------------------------------------------------------------------
On 2017-11-14T16:55:01+00:00 Bugzilla-x wrote:
Created attachment 363618
lib: Remove unneeded include
Nothing is using the code in the generated bluetooth-fdo-glue.h
header.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/9
------------------------------------------------------------------------
On 2017-11-14T16:55:07+00:00 Bugzilla-x wrote:
Created attachment 363619
lib: Remove never used FDO_PROPERTIES_INTERFACE constant
It was added as a constant during the BlueZ 5 port (commit 0b05349) but
was never used.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/10
------------------------------------------------------------------------
On 2017-11-14T16:55:13+00:00 Bugzilla-x wrote:
Created attachment 363620
lib: Indent and document gtk_tree_store_new() call
Makes it easier to see which column has which type.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/11
------------------------------------------------------------------------
On 2017-11-14T16:55:18+00:00 Bugzilla-x wrote:
Created attachment 363621
lib: Use GDBusObjectManager
Instead of our home-made ObjectManager proxy. Note that this is a
minimal port and will require more cleaning up to be up to standards.
Loosely based on patch by Benjamin Berg <bberg at redhat.com>
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/12
------------------------------------------------------------------------
On 2017-11-14T16:55:24+00:00 Bugzilla-x wrote:
Created attachment 363622
lib: Remove creation of intermediate GDBusProxies
We already have what we need created by the GDBusObjectManager.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/13
------------------------------------------------------------------------
On 2017-11-14T16:55:29+00:00 Bugzilla-x wrote:
Created attachment 363623
lib: Simplify Properties setting
Stop setting properties on adapters or devices by making D-Bus calls
ourselves, and rely on wrapped GObject properties instead.
For the calls we're making, the only change will be the error message
coming from the generated wrapper, rather than being our own custom ones.
We weren't really able to recover from errors in the past anyway, so
this shouldn't make the overall experience any worse.
This also fixes a possible hang on exit in the Bluetooth panel.
See https://bugzilla.gnome.org/show_bug.cgi?id=789654
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/14
------------------------------------------------------------------------
On 2017-11-14T16:55:35+00:00 Bugzilla-x wrote:
Created attachment 363624
lib: Remove last users of "Properties" D-Bus wrapper
As those property objects were not used anywhere anymore.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/15
------------------------------------------------------------------------
On 2017-11-14T16:55:41+00:00 Bugzilla-x wrote:
Created attachment 363625
lib: Remove fd.o Properties API glue
As we removed the last users of that code.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/16
------------------------------------------------------------------------
On 2017-11-14T16:55:47+00:00 Bugzilla-x wrote:
Created attachment 363626
lib: Fix possible race when creating adapters or devices
When new D-Bus objects appear, listen to changes to their properties as
soon as possible to avoid missing out on events. This can happen if a
synchronous D-Bus call is made between the device appearing, and we
start listening to changes.
The window for that race is smaller now that we avoid creating
GDBusProxies and fetching properties synchronously when new adapters or
devices appear, but this close that window altogether.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/17
------------------------------------------------------------------------
On 2017-11-14T16:55:52+00:00 Bugzilla-x wrote:
Created attachment 363627
lib: Always send "default-adapter-powered"
Fix sending of default-adapter-powered in the case that the default
adapter is being disabled.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/18
------------------------------------------------------------------------
On 2017-11-14T16:58:35+00:00 Bugzilla-x wrote:
Attachment 363618 pushed as b5a6455 - lib: Remove unneeded include
Attachment 363619 pushed as 899eb7d - lib: Remove never used FDO_PROPERTIES_INTERFACE constant
Attachment 363620 pushed as 215acf6 - lib: Indent and document gtk_tree_store_new() call
Attachment 363621 pushed as 17983ac - lib: Use GDBusObjectManager
Attachment 363622 pushed as 9319001 - lib: Remove creation of intermediate GDBusProxies
Attachment 363623 pushed as 27e88fc - lib: Simplify Properties setting
Attachment 363624 pushed as 61f5ad5 - lib: Remove last users of "Properties" D-Bus wrapper
Attachment 363625 pushed as afccb57 - lib: Remove fd.o Properties API glue
Attachment 363626 pushed as 9c6d93a - lib: Fix possible race when creating adapters or devices
Attachment 363627 pushed as dbe815a - lib: Always send "default-adapter-powered"
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/19
------------------------------------------------------------------------
On 2017-11-14T17:01:50+00:00 Bugzilla-x wrote:
Which leaves us with attachment 351968. I have trouble understanding in
which case this could happen now. Can you file a new bug if this is
still needed?
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/20
------------------------------------------------------------------------
On 2017-11-15T10:45:24+00:00 BenjaminBerg wrote:
Oh, right. Attachment 351968 was just my way of to not duplicate the
icon override logic.
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/21
------------------------------------------------------------------------
On 2018-02-15T12:52:02+00:00 BenjaminBerg wrote:
*** Bug 775376 has been marked as a duplicate of this bug. ***
Reply at: https://bugs.launchpad.net/ubuntu/+source/gnome-
shell/+bug/1738838/comments/25
** Changed in: gnome-bluetooth
Status: Unknown => Fix Released
** Changed in: gnome-bluetooth
Importance: Unknown => Medium
** Bug watch added: bugzilla.gnome.org/ #789654
https://bugzilla.gnome.org/show_bug.cgi?id=789654
** Changed in: gnome-shell
Status: Unknown => Expired
** Changed in: gnome-shell
Importance: Unknown => Medium
--
You received this bug notification because you are a member of
Bluetooth, which is subscribed to gnome-bluetooth in Ubuntu.
https://bugs.launchpad.net/bugs/1738838
Title:
Bluetooth icon shows "Off" and the only option given below it is "Turn
Off"
To manage notifications about this bug go to:
https://bugs.launchpad.net/gnome-bluetooth/+bug/1738838/+subscriptions
More information about the Ubuntu-bluetooth
mailing list