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

As of 5.73: rfkill block bluetooth puts bluetoothd in loop at 100% CPU. #785

Closed
dimitris-personal opened this issue Mar 21, 2024 · 10 comments

Comments

@dimitris-personal
Copy link

dimitris-personal commented Mar 21, 2024

Summary: With at least one device connected, disabling bluetooth (e.g. airplane mode) gets stuck and never completes, requires a SIGKILL to bluetoothd.

This happens with the current latest code in master at 87cabb2 and also at the most recent release, 5.73.

Steps to reproduce

  • Fedora 39 Workstation GNOME, bluetooth active.
  • Connect at least one device. Tried this with either one of: Logitech MX Master 3, Google Pixel Buds Pro.
  • Run rfkill block bluetooth
  • bluetoothd takes a whole core, GNOME quick settings and status still shows bluetooth as "active". I have to kill -9 the process to get the block to really complete.
  • Turning off the connected device does not break the loop.

While pegging the CPU, this seems to be the stack (this is with the second of the builds above):

(gdb) bt
#0  adapter_remove_connection (adapter=0x55a17e6889d0, device=0x55a17e698d30, bdaddr_type=2 '\002') at src/adapter.c:7476
#1  0x000055a17e55979f in adapter_stop (adapter=0x55a17e6889d0) at src/adapter.c:7527
#2  settings_changed (settings=<optimized out>, adapter=0x55a17e6889d0) at src/adapter.c:622
#3  new_settings_callback (index=<optimized out>, length=<optimized out>, param=<optimized out>, user_data=0x55a17e6889d0) at src/adapter.c:705
#4  0x000055a17e59981e in queue_foreach (user_data=0x7ffe49a7ef20, function=0x55a17e591e60 <notify_handler>, queue=0x55a17e683280) at src/shared/queue.c:207
#5  queue_foreach (user_data=0x7ffe49a7ef20, function=0x55a17e591e60 <notify_handler>, queue=0x55a17e683280) at src/shared/queue.c:190
#6  process_notify (param=<optimized out>, length=<optimized out>, index=<optimized out>, event=<optimized out>, mgmt=0x55a17e682f30) at src/shared/mgmt.c:349
#7  can_read_data (io=<optimized out>, user_data=0x55a17e682f30) at src/shared/mgmt.c:409
#8  0x000055a17e5bb679 in watch_callback (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>) at src/shared/io-glib.c:157
#9  0x00007f876edd4e5c in g_main_context_dispatch_unlocked.lto_priv () from target:/lib64/libglib-2.0.so.0
#10 0x00007f876ee2ff18 in g_main_context_iterate_unlocked.isra () from target:/lib64/libglib-2.0.so.0
#11 0x00007f876edd6447 in g_main_loop_run () from target:/lib64/libglib-2.0.so.0
#12 0x000055a17e4f1d64 in mainloop_run () at src/shared/mainloop-glib.c:66
#13 mainloop_run_with_signal (func=0x55a17e544740 <signal_callback>, user_data=0x0) at src/shared/mainloop-notify.c:188
#14 main (argc=<optimized out>, argv=<optimized out>) at src/main.c:1455
@dimitris-personal
Copy link
Author

dimitris-personal commented Mar 22, 2024

If my gdb/C skills haven't atrophied too much over the years, this seems to be happening:

The loop at:

while (adapter->connections) {

becomes infinite. This test inside adapter_remove_connection:

if (btd_device_is_connected(device))

always returns true, so the list is never advanced:

adapter->connections = g_slist_remove(adapter->connections, device);

@dimitris-personal
Copy link
Author

Reverting 44d3f67 fixes this.

dimitris-personal referenced this issue Mar 22, 2024
This checks if there is any service connected on device_is_connected
since some profiles maybe probed using advertising data which doesn't
require a connection.
@Cuttergott
Copy link

Issue replicated on endeavourOS 6.8.1-arch1-1, bluez 5.73. Exactly same behaviour

@pv
Copy link
Contributor

pv commented Mar 27, 2024

Afaik this was fixed in 8060d12

@dimitris-personal
Copy link
Author

@pv unfortunately running either 8060d12 or current master at 41d6c4e still reproduces the problem for me.

master with this added:

diff --git a/src/device.c b/src/device.c
index 5e74633c6..de5f94c7d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3273,11 +3273,7 @@ uint8_t btd_device_get_bdaddr_type(struct btd_device *dev)
 
 bool btd_device_is_connected(struct btd_device *dev)
 {
-       if (dev->bredr_state.connected || dev->le_state.connected)
-               return true;
-
-       return find_service_with_state(dev->services,
-                                               BTD_SERVICE_STATE_CONNECTED);
+       return dev->bredr_state.connected || dev->le_state.connected;
 }
 
 static void clear_temporary_timer(struct btd_device *dev)

resolves the issue.

@dimitris-personal
Copy link
Author

device_disappeared does not appear to be called (breakpoint not reached for me) on rfkill block bluetooth so makes sense that 8060d12 wouldn't fix this.

@bsmojver
Copy link

bsmojver commented Apr 3, 2024

@dimitris-personal, did you want to open a PR for your patch above, so that it can be officially reviewed/committed?

@dimitris-personal
Copy link
Author

Created a PR #802

@dimitris-personal
Copy link
Author

Just for reference, and since github PRs are not the official change submission channel (can you tell I'm a newbie in this?), I've submitted an updated, narrower change in the mailing list. Both changes are in this thread, this narrower one is in the V2 part of the thread:

diff --git a/src/adapter.c b/src/adapter.c
index 4bcc464de..0b7aab4b5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -7486,7 +7486,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter,
                device_cancel_authentication(device, TRUE);
 
        /* If another bearer is still connected */
-       if (btd_device_is_connected(device))
+       if (btd_device_state_is_connected(device))
                return;
 
        adapter->connections = g_slist_remove(adapter->connections, device);
diff --git a/src/device.c b/src/device.c
index 5e74633c6..123b1b796 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3273,13 +3273,18 @@ uint8_t btd_device_get_bdaddr_type(struct btd_device *dev)
 
 bool btd_device_is_connected(struct btd_device *dev)
 {
-       if (dev->bredr_state.connected || dev->le_state.connected)
+       if (btd_device_state_is_connected(dev))
                return true;
 
        return find_service_with_state(dev->services,
                                                BTD_SERVICE_STATE_CONNECTED);
 }
 
+bool btd_device_state_is_connected(struct btd_device *dev)
+{
+       return dev->bredr_state.connected || dev->le_state.connected;
+}
+
 static void clear_temporary_timer(struct btd_device *dev)
 {
        if (dev->temporary_timer) {
diff --git a/src/device.h b/src/device.h
index d4e70b7ef..e3191f2a4 100644
--- a/src/device.h
+++ b/src/device.h
@@ -104,6 +104,7 @@ void device_set_rssi(struct btd_device *device, int8_t rssi);
 void device_set_tx_power(struct btd_device *device, int8_t tx_power);
 void device_set_flags(struct btd_device *device, uint8_t flags);
 bool btd_device_is_connected(struct btd_device *dev);
+bool btd_device_state_is_connected(struct btd_device *dev);
 uint8_t btd_device_get_bdaddr_type(struct btd_device *dev);
 bool device_is_retrying(struct btd_device *device);
 void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,

@bsmojver
Copy link

bsmojver commented Apr 5, 2024

Thank you @dimitris-personal !

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

Successfully merging a pull request may close this issue.

4 participants