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

4 files changed, 14 insertions, 7 deletions

diff --git a/system/rust/Android.bp b/system/rust/Android.bp
index e1b9caa91d..dd24b57591 100644
--- a/system/rust/Android.bp
+++ b/system/rust/Android.bp
@@ -39,7 +39,6 @@ rust_defaults {
         "libbt_common_only_init_flags",
         "libcxx",
         "liblog_rust",
-        "libonce_cell",
         "libscopeguard",
 
         // needed to work around duplicate symbols
diff --git a/system/rust/Cargo.toml b/system/rust/Cargo.toml
index 87429e132c..3067f27d1e 100644
--- a/system/rust/Cargo.toml
+++ b/system/rust/Cargo.toml
@@ -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"]
diff --git a/system/rust/src/gatt/arbiter.rs b/system/rust/src/gatt/arbiter.rs
index 156d0c9085..52a0aa7207 100644
--- a/system/rust/src/gatt/arbiter.rs
+++ b/system/rust/src/gatt/arbiter.rs
@@ -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
diff --git a/system/rust/src/lib.rs b/system/rust/src/lib.rs
index 7e628608a5..4693320516 100644
--- a/system/rust/src/lib.rs
+++ b/system/rust/src/lib.rs
@@ -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();
     }
 }