Use Stack Handler for all Module Init

Change-Id: I7ec0e5017b9004e9a5e82ca4581d730438200129
Test: m com.android.bt
Flag: com::android::bluetooth::flags::same_handler_for_all_modules
Bug: 393449774
Bug: 398195386

15 files changed, 59 insertions, 32 deletions

diff --git a/system/gd/hal/hci_hal_android_test.cc b/system/gd/hal/hci_hal_android_test.cc
index 28ce145379..471e54bb70 100644
--- a/system/gd/hal/hci_hal_android_test.cc
+++ b/system/gd/hal/hci_hal_android_test.cc
@@ -74,12 +74,15 @@ class HciHalAndroidTest : public ::testing::Test {
 protected:
   void SetUp() override {
     thread_ = new Thread("test_thread", Thread::Priority::NORMAL);
-    hal = fake_registry_.Start<HciHal>(thread_);
+    handler_ = new os::Handler(thread_);
+    hal = fake_registry_.Start<HciHal>(thread_, handler_);
   }
 
   void TearDown() override {
     fake_registry_.StopAll();
+    handler_->Clear();
     delete thread_;
+    delete handler_;
   }
 
   HciHal* hal;
@@ -87,6 +90,7 @@ protected:
 private:
   ModuleRegistry fake_registry_;
   Thread* thread_;
+  os::Handler* handler_;
 };
 
 TEST_F(HciHalAndroidTest, init) {
diff --git a/system/gd/hal/hci_hal_host_test.cc b/system/gd/hal/hci_hal_host_test.cc
index ab0ccef8f6..9f50867f59 100644
--- a/system/gd/hal/hci_hal_host_test.cc
+++ b/system/gd/hal/hci_hal_host_test.cc
@@ -141,10 +141,11 @@ class HciHalRootcanalTest : public ::testing::Test {
 protected:
   void SetUp() override {
     thread_ = new Thread("test_thread", Thread::Priority::NORMAL);
+    handler_ = new os::Handler(thread_);
 
     HciHalHostRootcanalConfig::Get()->SetPort(kTestPort);
     fake_server_ = new FakeRootcanalDesktopHciServer;
-    hal_ = fake_registry_.Start<HciHal>(thread_);
+    hal_ = fake_registry_.Start<HciHal>(thread_, handler_);
     hal_->registerIncomingPacketCallback(&callbacks_);
     fake_server_socket_ =
             fake_server_->Accept();  // accept() after client is connected to avoid blocking
@@ -156,8 +157,10 @@ protected:
     hal_->unregisterIncomingPacketCallback();
     fake_registry_.StopAll();
     close(fake_server_socket_);
+    handler_->Clear();
     delete fake_server_;
     delete thread_;
+    delete handler_;
   }
 
   void SetFakeServerSocketToBlocking() {
@@ -172,6 +175,7 @@ protected:
   TestHciHalCallbacks callbacks_;
   int fake_server_socket_ = -1;
   Thread* thread_;
+  os::Handler* handler_;
 };
 
 void check_packet_equal(std::pair<uint8_t, HciPacket> hci_packet1_type_data_pair,
diff --git a/system/gd/hci/acl_manager/acl_scheduler_test.cc b/system/gd/hci/acl_manager/acl_scheduler_test.cc
index afdddcd3a7..fa5b0fc164 100644
--- a/system/gd/hci/acl_manager/acl_scheduler_test.cc
+++ b/system/gd/hci/acl_manager/acl_scheduler_test.cc
@@ -52,7 +52,7 @@ MATCHER(IsSet, "Future is set") {
 class AclSchedulerTest : public ::testing::Test {
 protected:
   void SetUp() override {
-    fake_registry_.Start<AclScheduler>(&thread_);
+    fake_registry_.Start<AclScheduler>(&thread_, fake_registry_.GetTestHandler());
     ASSERT_TRUE(fake_registry_.IsStarted<AclScheduler>());
 
     client_handler_ = fake_registry_.GetTestModuleHandler(&AclScheduler::Factory);
diff --git a/system/gd/hci/acl_manager_test.cc b/system/gd/hci/acl_manager_test.cc
index c4f7deb89e..e0a1c6674a 100644
--- a/system/gd/hci/acl_manager_test.cc
+++ b/system/gd/hci/acl_manager_test.cc
@@ -119,7 +119,7 @@ protected:
     ASSERT_NE(client_handler_, nullptr);
     bluetooth::hci::testing::mock_storage_ = new storage::StorageModule(new os::Handler(&thread_));
     bluetooth::hci::testing::mock_storage_->Start();
-    fake_registry_.Start<AclManager>(&thread_);
+    fake_registry_.Start<AclManager>(&thread_, handler_);
     acl_manager_ =
             static_cast<AclManager*>(fake_registry_.GetModuleUnderTest(&AclManager::Factory));
     Address::FromString("A1:A2:A3:A4:A5:A6", remote);
@@ -173,6 +173,7 @@ protected:
   HciLayerFake* test_hci_layer_ = nullptr;
   TestController* test_controller_ = nullptr;
   os::Thread& thread_ = fake_registry_.GetTestThread();
+  os::Handler* handler_ = fake_registry_.GetTestHandler();
   AclManager* acl_manager_ = nullptr;
   os::Handler* client_handler_ = nullptr;
   Address remote;
@@ -1146,7 +1147,7 @@ protected:
     ASSERT_NE(client_handler_, nullptr);
     bluetooth::hci::testing::mock_storage_ = new storage::StorageModule(new os::Handler(&thread_));
     bluetooth::hci::testing::mock_storage_->Start();
-    fake_registry_.Start<AclManager>(&thread_);
+    fake_registry_.Start<AclManager>(&thread_, handler_);
     acl_manager_ =
             static_cast<AclManager*>(fake_registry_.GetModuleUnderTest(&AclManager::Factory));
     Address::FromString("A1:A2:A3:A4:A5:A6", remote);
diff --git a/system/gd/hci/acl_manager_unittest.cc b/system/gd/hci/acl_manager_unittest.cc
index 797b5aed26..bfb132594c 100644
--- a/system/gd/hci/acl_manager_unittest.cc
+++ b/system/gd/hci/acl_manager_unittest.cc
@@ -155,7 +155,7 @@ protected:
     fake_registry_.InjectTestModule(&Controller::Factory, test_controller_);
     client_handler_ = fake_registry_.GetTestModuleHandler(&HciLayer::Factory);
     ASSERT_NE(client_handler_, nullptr);
-    fake_registry_.Start<AclManager>(&thread_);
+    fake_registry_.Start<AclManager>(&thread_, fake_registry_.GetTestHandler());
   }
 
   void TearDown() override {
@@ -440,7 +440,7 @@ protected:
     fake_registry_.InjectTestModule(&Controller::Factory, test_controller_);
     client_handler_ = fake_registry_.GetTestModuleHandler(&HciLayer::Factory);
     ASSERT_NE(client_handler_, nullptr);
-    fake_registry_.Start<AclManager>(&thread_);
+    fake_registry_.Start<AclManager>(&thread_, fake_registry_.GetTestHandler());
     acl_manager_ =
             static_cast<AclManager*>(fake_registry_.GetModuleUnderTest(&AclManager::Factory));
     hci::Address address;
diff --git a/system/gd/hci/controller_test.cc b/system/gd/hci/controller_test.cc
index 0743204ffd..fca2d3e9e2 100644
--- a/system/gd/hci/controller_test.cc
+++ b/system/gd/hci/controller_test.cc
@@ -291,7 +291,7 @@ protected:
     vendor_capabilities_.reset();
     fake_registry_.InjectTestModule(&HciLayer::Factory, test_hci_layer_);
     client_handler_ = fake_registry_.GetTestModuleHandler(&HciLayer::Factory);
-    fake_registry_.Start<Controller>(&thread_);
+    fake_registry_.Start<Controller>(&thread_, fake_registry_.GetTestHandler());
     controller_ = static_cast<Controller*>(fake_registry_.GetModuleUnderTest(&Controller::Factory));
   }
 
diff --git a/system/gd/hci/hci_layer_test.cc b/system/gd/hci/hci_layer_test.cc
index 1f677d518b..c30094d292 100644
--- a/system/gd/hci/hci_layer_test.cc
+++ b/system/gd/hci/hci_layer_test.cc
@@ -210,7 +210,8 @@ public:
     hal = new hal::TestHciHal();
 
     fake_registry_.InjectTestModule(&hal::HciHal::Factory, hal);
-    fake_registry_.Start<DependsOnHci>(&fake_registry_.GetTestThread());
+    fake_registry_.Start<DependsOnHci>(&fake_registry_.GetTestThread(),
+                                       fake_registry_.GetTestHandler());
     hci = static_cast<HciLayer*>(fake_registry_.GetModuleUnderTest(&HciLayer::Factory));
     upper = static_cast<DependsOnHci*>(fake_registry_.GetModuleUnderTest(&DependsOnHci::Factory));
     ASSERT_TRUE(fake_registry_.IsStarted<HciLayer>());
diff --git a/system/gd/hci/hci_layer_unittest.cc b/system/gd/hci/hci_layer_unittest.cc
index 5abc0c74fc..86afb1298d 100644
--- a/system/gd/hci/hci_layer_unittest.cc
+++ b/system/gd/hci/hci_layer_unittest.cc
@@ -87,7 +87,8 @@ protected:
   void SetUp() override {
     hal_ = new hal::TestHciHal();
     fake_registry_.InjectTestModule(&hal::HciHal::Factory, hal_);
-    fake_registry_.Start<HciLayer>(&fake_registry_.GetTestThread());
+    fake_registry_.Start<HciLayer>(&fake_registry_.GetTestThread(),
+                                   fake_registry_.GetTestHandler());
     hci_ = static_cast<HciLayer*>(fake_registry_.GetModuleUnderTest(&HciLayer::Factory));
     hci_handler_ = fake_registry_.GetTestModuleHandler(&HciLayer::Factory);
     ASSERT_TRUE(fake_registry_.IsStarted<HciLayer>());
diff --git a/system/gd/hci/le_advertising_manager_test.cc b/system/gd/hci/le_advertising_manager_test.cc
index 0c33ab9e33..12fa70d263 100644
--- a/system/gd/hci/le_advertising_manager_test.cc
+++ b/system/gd/hci/le_advertising_manager_test.cc
@@ -181,7 +181,8 @@ protected:
     test_controller_->num_advertisers_ = num_instances_;
     test_controller_->vendor_capabilities_.max_advt_instances_ = num_instances_;
     test_controller_->SetBleExtendedAdvertisingSupport(support_ble_extended_advertising_);
-    le_advertising_manager_ = fake_registry_.Start<LeAdvertisingManager>(&thread_);
+    le_advertising_manager_ =
+            fake_registry_.Start<LeAdvertisingManager>(&thread_, fake_registry_.GetTestHandler());
     le_advertising_manager_->RegisterAdvertisingCallback(&mock_advertising_callback_);
   }
 
diff --git a/system/gd/hci/le_scanning_manager_test.cc b/system/gd/hci/le_scanning_manager_test.cc
index 7eab705d0f..134702cb4a 100644
--- a/system/gd/hci/le_scanning_manager_test.cc
+++ b/system/gd/hci/le_scanning_manager_test.cc
@@ -288,7 +288,7 @@ protected:
   }
 
   void start_le_scanning_manager() {
-    fake_registry_.Start<LeScanningManager>(&thread_);
+    fake_registry_.Start<LeScanningManager>(&thread_, fake_registry_.GetTestHandler());
     le_scanning_manager = static_cast<LeScanningManager*>(
             fake_registry_.GetModuleUnderTest(&LeScanningManager::Factory));
     le_scanning_manager->RegisterScanningCallback(&mock_callbacks_);
diff --git a/system/gd/hci/remote_name_request_test.cc b/system/gd/hci/remote_name_request_test.cc
index a90ff2eb47..43367e89be 100644
--- a/system/gd/hci/remote_name_request_test.cc
+++ b/system/gd/hci/remote_name_request_test.cc
@@ -67,7 +67,7 @@ protected:
     test_hci_layer_ = new HciLayerFake;
     fake_registry_.InjectTestModule(&HciLayer::Factory, test_hci_layer_);
 
-    fake_registry_.Start<RemoteNameRequestModule>(&thread_);
+    fake_registry_.Start<RemoteNameRequestModule>(&thread_, fake_registry_.GetTestHandler());
     ASSERT_TRUE(fake_registry_.IsStarted<RemoteNameRequestModule>());
 
     client_handler_ = fake_registry_.GetTestModuleHandler(&RemoteNameRequestModule::Factory);
diff --git a/system/gd/module.cc b/system/gd/module.cc
index ba7695db28..d5ea4c4fd3 100644
--- a/system/gd/module.cc
+++ b/system/gd/module.cc
@@ -66,18 +66,26 @@ bool ModuleRegistry::IsStarted(const ModuleFactory* module) const {
   return started_modules_.find(module) != started_modules_.end();
 }
 
-void ModuleRegistry::Start(ModuleList* modules, Thread* thread) {
+void ModuleRegistry::Start(ModuleList* modules, Thread* thread, Handler* handler) {
   for (auto it = modules->list_.begin(); it != modules->list_.end(); it++) {
-    Start(*it, thread);
+    Start(*it, thread, handler);
   }
 }
 
-void ModuleRegistry::set_registry_and_handler(Module* instance, Thread* thread) const {
+void ModuleRegistry::set_registry_and_handler(Module* instance, Thread* thread,
+                                              Handler* handler) const {
   instance->registry_ = this;
-  instance->handler_ = new Handler(thread);
+
+  if (com::android::bluetooth::flags::same_handler_for_all_modules()) {
+    // Use same handler for all modules initialization.
+    // TODO: remove the dependency on the `thread` when the flag is removed.
+    instance->handler_ = handler;
+  } else {
+    instance->handler_ = new Handler(thread);
+  }
 }
 
-Module* ModuleRegistry::Start(const ModuleFactory* module, Thread* thread) {
+Module* ModuleRegistry::Start(const ModuleFactory* module, Thread* thread, Handler* handler) {
   {
     std::unique_lock<std::mutex> lock(started_modules_guard_, std::defer_lock);
     if (com::android::bluetooth::flags::fix_started_module_race()) {
@@ -91,11 +99,11 @@ Module* ModuleRegistry::Start(const ModuleFactory* module, Thread* thread) {
 
   log::info("Constructing next module");
   Module* instance = module->ctor_();
-  set_registry_and_handler(instance, thread);
+  set_registry_and_handler(instance, thread, handler);
 
   log::info("Starting dependencies of {}", instance->ToString());
   instance->ListDependencies(&instance->dependencies_);
-  Start(&instance->dependencies_, thread);
+  Start(&instance->dependencies_, thread, handler);
 
   log::info("Finished starting dependencies and calling Start() of {}", instance->ToString());
 
diff --git a/system/gd/module.h b/system/gd/module.h
index 28bebe568b..b53f992ccf 100644
--- a/system/gd/module.h
+++ b/system/gd/module.h
@@ -140,14 +140,14 @@ public:
 
   // Start all the modules on this list and their dependencies
   // in dependency order
-  void Start(ModuleList* modules, ::bluetooth::os::Thread* thread);
+  void Start(ModuleList* modules, ::bluetooth::os::Thread* thread, os::Handler* handler);
 
   template <class T>
-  T* Start(::bluetooth::os::Thread* thread) {
-    return static_cast<T*>(Start(&T::Factory, thread));
+  T* Start(::bluetooth::os::Thread* thread, os::Handler* handler) {
+    return static_cast<T*>(Start(&T::Factory, thread, handler));
   }
 
-  Module* Start(const ModuleFactory* id, ::bluetooth::os::Thread* thread);
+  Module* Start(const ModuleFactory* id, ::bluetooth::os::Thread* thread, os::Handler* handler);
 
   // Stop all running modules in reverse order of start
   void StopAll();
@@ -155,7 +155,8 @@ public:
 protected:
   Module* Get(const ModuleFactory* module) const;
 
-  void set_registry_and_handler(Module* instance, ::bluetooth::os::Thread* thread) const;
+  void set_registry_and_handler(Module* instance, ::bluetooth::os::Thread* thread,
+                                os::Handler* handler) const;
 
   os::Handler* GetModuleHandler(const ModuleFactory* module) const;
 
@@ -172,7 +173,7 @@ public:
   void InjectTestModule(const ModuleFactory* module, Module* instance) {
     start_order_.push_back(module);
     started_modules_[module] = instance;
-    set_registry_and_handler(instance, &test_thread);
+    set_registry_and_handler(instance, &test_thread, test_handler_);
     instance->Start();
   }
 
@@ -188,6 +189,7 @@ public:
   }
 
   os::Thread& GetTestThread() { return test_thread; }
+  os::Handler* GetTestHandler() { return test_handler_; }
 
   bool SynchronizeModuleHandler(const ModuleFactory* module,
                                 std::chrono::milliseconds timeout) const {
@@ -203,6 +205,7 @@ public:
 
 private:
   os::Thread test_thread{"test_thread", os::Thread::Priority::NORMAL};
+  os::Handler* test_handler_ = new os::Handler(&test_thread);
 };
 
 class FuzzTestModuleRegistry : public TestModuleRegistry {
@@ -216,7 +219,7 @@ public:
 
   template <class T>
   T* Start() {
-    return ModuleRegistry::Start<T>(&GetTestThread());
+    return ModuleRegistry::Start<T>(&GetTestThread(), GetTestHandler());
   }
 
   void WaitForIdleAndStopAll() {
diff --git a/system/gd/module_unittest.cc b/system/gd/module_unittest.cc
index 24ccc8b3d2..d89be74219 100644
--- a/system/gd/module_unittest.cc
+++ b/system/gd/module_unittest.cc
@@ -35,16 +35,20 @@ class ModuleTest : public ::testing::Test {
 protected:
   void SetUp() override {
     thread_ = new Thread("test_thread", Thread::Priority::NORMAL);
+    handler_ = new os::Handler(thread_);
     registry_ = new ModuleRegistry();
   }
 
   void TearDown() override {
+    handler_->Clear();
     delete registry_;
     delete thread_;
+    delete handler_;
   }
 
   ModuleRegistry* registry_;
   Thread* thread_;
+  os::Handler* handler_;
 };
 
 os::Handler* test_module_no_dependency_handler = nullptr;
@@ -161,7 +165,7 @@ const ModuleFactory TestModuleTwoDependencies::Factory =
 TEST_F(ModuleTest, no_dependency) {
   ModuleList list;
   list.add<TestModuleNoDependency>();
-  registry_->Start(&list, thread_);
+  registry_->Start(&list, thread_, handler_);
 
   EXPECT_TRUE(registry_->IsStarted<TestModuleNoDependency>());
   EXPECT_FALSE(registry_->IsStarted<TestModuleOneDependency>());
@@ -179,7 +183,7 @@ TEST_F(ModuleTest, no_dependency) {
 TEST_F(ModuleTest, one_dependency) {
   ModuleList list;
   list.add<TestModuleOneDependency>();
-  registry_->Start(&list, thread_);
+  registry_->Start(&list, thread_, handler_);
 
   EXPECT_TRUE(registry_->IsStarted<TestModuleNoDependency>());
   EXPECT_TRUE(registry_->IsStarted<TestModuleOneDependency>());
@@ -197,7 +201,7 @@ TEST_F(ModuleTest, one_dependency) {
 TEST_F(ModuleTest, two_dependencies) {
   ModuleList list;
   list.add<TestModuleTwoDependencies>();
-  registry_->Start(&list, thread_);
+  registry_->Start(&list, thread_, handler_);
 
   EXPECT_TRUE(registry_->IsStarted<TestModuleNoDependency>());
   EXPECT_TRUE(registry_->IsStarted<TestModuleOneDependency>());
@@ -220,7 +224,7 @@ void post_to_module_one_handler() {
 TEST_F(ModuleTest, shutdown_with_unhandled_callback) {
   ModuleList list;
   list.add<TestModuleOneDependency>();
-  registry_->Start(&list, thread_);
+  registry_->Start(&list, thread_, handler_);
   test_module_no_dependency_handler->Post(common::BindOnce(&post_to_module_one_handler));
   registry_->StopAll();
 }
diff --git a/system/main/shim/stack.cc b/system/main/shim/stack.cc
index c570a5f542..14447d75b3 100644
--- a/system/main/shim/stack.cc
+++ b/system/main/shim/stack.cc
@@ -259,7 +259,7 @@ void Stack::handle_start_up(ModuleList* modules, std::promise<void> promise) {
   pimpl_->counter_metrics_->Start();
   pimpl_->storage_->Start();
   pimpl_->snoop_logger_->Start();
-  registry_.Start(modules, stack_thread_);
+  registry_.Start(modules, stack_thread_, stack_handler_);
   promise.set_value();
 }