Race condition causes panic on subscription in CoreBluetooth

[D0ntPanic]

Describe the bug I'm getting panics after calling subscribe on a characteristic on CoreBluetooth for a very specific device (the GoCube smart cube).

It appears that what is happening is that extra characteristic discovery events are firing after connect completes, which causes race conditions in the subscription code. If you attempt to subscribe to the data characteristic on the GoCube, often an extra characteristic discovery event fires before the subscription completes and wipes out the existing characteristic data. When the subscription request completes, an attempt is made to complete the subscription request in the subscribe_future_state field of the characteristic. Unfortunately the extra discovery event has cleared this field, so a panic results.

Expected behavior The subscription completes and starts sending notification events.

Actual behavior A panic crashes the thread and all Bluetooth connections stop working.

The panic happens on line 417 in src/corebluetooth/internal.rs:

let state = c.subscribe_future_state.pop_back().unwrap();

Additional context I have only seen this on the GoCube smart cube and it's sister product the Rubik's Connected smart cube.

I made a simple patch to fix it from panicking, but this is likely not a real solution to the underlying issue. I don't know the CoreBluetooth API well enough to know the true root cause. The commit which fixes the issue for me (but is likely not good enough to integrate) is here:

https://github.com/D0ntPanic/btleplug/commit/1bfb34b514688908aaaf13273dcd23724d10a30a

[qdot]

@D0ntPanic You might want to give the dev branch a shot, though you'll need some extra patches on top of it (See #161). That's been far more stable on macOS for me. I'm hoping we can get this released sometime soon, we're just blocked on some linux issues right now.

[D0ntPanic]

I wrote a minimized repro using the dev channel with the extra patches, and the bug still exists in latest.

Minimal repro code:

use btleplug::api::{BDAddr, Central, Peripheral, Manager as _};
use btleplug::platform::{Adapter, Manager};
use futures::StreamExt;
use std::time::Duration;
use uuid::Uuid;
use std::str::FromStr;

#[tokio::main]
async fn main() {
    let manager = Manager::new().await.unwrap();
    let adapter = manager.adapters().await.unwrap();
    let central = adapter
        .into_iter()
        .nth(0)
        .unwrap();
    central.start_scan().await.unwrap();

    loop {
        // Enumerate devices
        for device in central.peripherals().await.unwrap() {
            if let Some(name) = device.properties().await.unwrap().local_name {
                if name.starts_with("GoCube") {
                    println!("Found GoCube, connecting");
                    device.connect().await.unwrap();

                    let characteristics = device.discover_characteristics().await.unwrap();

                    let mut read = None;
                    for characteristic in characteristics {
                        if characteristic.uuid
                            == Uuid::from_str("6e400003-b5a3-f393-e0a9-e50e24dcca9e").unwrap()
                        {
                            read = Some(characteristic);
                        }
                    }
                    let read = read.unwrap();

                    println!("Subscribing");
                    device.subscribe(&read).await.unwrap();
                    let mut notifications = device.notifications().await.unwrap();
                    println!("Watching for events");
                    while let Some(value) = notifications.next().await {
                        println!("{:?}", value);
                    }
                    return;
                }
            }
        }

        // This is technically wrong in async but this is just a test
        std::thread::sleep(Duration::from_millis(100));
    }
}

Output:

Found GoCube, connecting
Subscribing
thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /Users/rwagner/projects/btleplug/src/corebluetooth/internal.rs:410:65
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The line of code at fault is the same, so it is likely the same root cause.

Applying the same patch as in original post to latest code gives the expected results:

Found GoCube, connecting
Subscribing
Watching for events
ValueNotification { uuid: 6e400003-b5a3-f393-e0a9-e50e24dcca9e, value: [42, 6, 1, 4, 9, 62, 13, 10] }
ValueNotification { uuid: 6e400003-b5a3-f393-e0a9-e50e24dcca9e, value: [42, 6, 1, 5, 6, 60, 13, 10] }

Unfortunately the repro requires access to a GoCube or a Rubik's Connected cube.