Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 723407a1 authored by William Escande's avatar William Escande
Browse files

Gatt arbiter: use RwLock to allow global cleanup 

The test for this code is flaky and never works on the first iteration
but works on the second attempt.
This is because the crash is cleaning the bluetooth stack and let the
arbiter be set a second time.
The proper fix is to clean the arbiter when the stack is shut down, but
being a OnceCell prevent it.
Option 1 to solve it is to no longer use a OnceCell but a optional
value that will require a lock to be shared across multiples caller.
Option 2 is to have a mutable OnceCell but this looks like an
anti-pattern

The cost of this CL is an added rwlock when doing a call on the arbiter

Bug: 287403942
Test: atest net_test_bluetooth.GattTest.GattServerBuild
Change-Id: Ibe70e51dcdade64570ff95894aad21f5bd35a109
parent 0b3fe155

system/rust/Android.bp

+0 −1
Original line number Diff line number Diff line
@@ -39,7 +39,6 @@ rust_defaults { 
        "libbt_common_only_init_flags",

        "libcxx",

        "liblog_rust",

        "libonce_cell",

        "libscopeguard",



        // needed to work around duplicate symbols

system/rust/Cargo.toml

+0 −1
Original line number Diff line number Diff line
@@ -34,7 +34,6 @@ async-trait = "*" 
tokio-test = "0.4.2"

tokio = { version = "1.23.0", features = ["macros"] }

scopeguard = "1.1.0"

once_cell = "1.17.1"



[lib]

crate-type = ["rlib"]

system/rust/src/gatt/arbiter.rs

+12 −4
Original line number Diff line number Diff line
@@ -4,7 +4,7 @@ 
use std::sync::{Arc, Mutex};



use log::{error, trace};

use once_cell::sync::OnceCell;

use std::sync::RwLock;



use crate::{

    do_in_rust_thread,
@@ -19,12 +19,14 @@ use super::{ 
    server::isolation_manager::IsolationManager,

};



static ARBITER: OnceCell<Arc<Mutex<IsolationManager>>> = OnceCell::new();

static ARBITER: RwLock<Option<Arc<Mutex<IsolationManager>>>> = RwLock::new(None);



/// Initialize the Arbiter

pub fn initialize_arbiter() -> Arc<Mutex<IsolationManager>> {

    let arbiter = Arc::new(Mutex::new(IsolationManager::new()));

    ARBITER.set(arbiter.clone()).unwrap_or_else(|_| panic!("Rust stack should only start up once"));

    let mut lock = ARBITER.write().unwrap();

    assert!(lock.is_none(), "Rust stack should only start up once");

    *lock = Some(arbiter.clone());



    StoreCallbacksFromRust(

        on_le_connect,
@@ -38,10 +40,16 @@ pub fn initialize_arbiter() -> Arc<Mutex<IsolationManager>> { 
    arbiter

}



/// Clean the Arbiter

pub fn clean_arbiter() {

    let mut lock = ARBITER.write().unwrap();

    *lock = None

}



/// Acquire the mutex holding the Arbiter and provide a mutable reference to the

/// supplied closure

pub fn with_arbiter<T>(f: impl FnOnce(&mut IsolationManager) -> T) -> T {

    f(ARBITER.get().unwrap().lock().as_mut().unwrap())

    f(ARBITER.read().unwrap().as_ref().expect("Rust stack is not started").lock().as_mut().unwrap())

}



/// Test to see if a buffer contains a valid ATT packet with an opcode we

system/rust/src/lib.rs

+2 −1
Original line number Diff line number Diff line
@@ -118,13 +118,14 @@ impl GlobalModuleRegistry { 
                match message {

                    MainThreadTxMessage::Callback(f) => f(&mut modules),

                    MainThreadTxMessage::Stop => {

                        GLOBAL_MODULE_REGISTRY.lock().unwrap().take();

                        break;

                    }

                }

            }

        });

        warn!("Rust thread queue has stopped, shutting down executor thread");

        GLOBAL_MODULE_REGISTRY.lock().unwrap().take();

        gatt::arbiter::clean_arbiter();

    }

}