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

Commit a30e4c43 authored by Hansong Zhang's avatar Hansong Zhang
Browse files

ModuleRegistry: Delete Module after all are stopped 

If a Module is stopped, it may still receive callback from lower level
Module, because the callback may have already been in lower level
Module's Handler queue. Instead of deleting the stopped Module
immediately, wait until all Modules are stopped.

Bug: 150174451
Test: bluetooth_test_gd
Change-Id: I0b55fe0ac7da09332d381d2192235339a9dc23fa
parent 5c1f674d

system/gd/module.cc

+4 −1
Original line number Diff line number Diff line
@@ -98,7 +98,10 @@ void ModuleRegistry::StopAll() { 
    instance->second->handler_->Clear();

    instance->second->handler_->WaitUntilStopped(kModuleStopTimeout);

    instance->second->Stop();



  }

  for (auto it = start_order_.rbegin(); it != start_order_.rend(); it++) {

    auto instance = started_modules_.find(*it);

    ASSERT(instance != started_modules_.end());

    delete instance->second->handler_;

    delete instance->second;

    started_modules_.erase(instance);

system/gd/module_unittest.cc

+23 −0
Original line number Diff line number Diff line
@@ -15,6 +15,8 @@ 
 */



#include "module.h"

#include "os/handler.h"

#include "os/thread.h"



#include "gtest/gtest.h"
@@ -39,6 +41,8 @@ class ModuleTest : public ::testing::Test { 
  Thread* thread_;

};



os::Handler* test_module_no_dependency_handler = nullptr;



class TestModuleNoDependency : public Module {

 public:

  static const ModuleFactory Factory;
@@ -50,10 +54,12 @@ class TestModuleNoDependency : public Module { 
  void Start() override {

    // A module is not considered started until Start() finishes

    EXPECT_FALSE(GetModuleRegistry()->IsStarted<TestModuleNoDependency>());

    test_module_no_dependency_handler = GetHandler();

  }



  void Stop() override {

    // A module is not considered stopped until after Stop() finishes

    test_module_no_dependency_handler = nullptr;

    EXPECT_TRUE(GetModuleRegistry()->IsStarted<TestModuleNoDependency>());

  }

};
@@ -62,6 +68,8 @@ const ModuleFactory TestModuleNoDependency::Factory = ModuleFactory([]() { 
  return new TestModuleNoDependency();

});



os::Handler* test_module_one_dependency_handler = nullptr;



class TestModuleOneDependency : public Module {

 public:

  static const ModuleFactory Factory;
@@ -76,9 +84,11 @@ class TestModuleOneDependency : public Module { 


    // A module is not considered started until Start() finishes

    EXPECT_FALSE(GetModuleRegistry()->IsStarted<TestModuleOneDependency>());

    test_module_one_dependency_handler = GetHandler();

  }



  void Stop() override {

    test_module_one_dependency_handler = GetHandler();

    EXPECT_TRUE(GetModuleRegistry()->IsStarted<TestModuleNoDependency>());



    // A module is not considered stopped until after Stop() finishes
@@ -199,5 +209,18 @@ TEST_F(ModuleTest, two_dependencies) { 
  EXPECT_FALSE(registry_->IsStarted<TestModuleTwoDependencies>());

}



void post_two_module_one_handler() {

  std::this_thread::sleep_for(std::chrono::milliseconds(100));

  test_module_one_dependency_handler->Post(common::BindOnce([] { FAIL(); }));

}



TEST_F(ModuleTest, shutdown_with_unhandled_callback) {

  ModuleList list;

  list.add<TestModuleOneDependency>();

  registry_->Start(&list, thread_);

  test_module_no_dependency_handler->Post(common::BindOnce(&post_two_module_one_handler));

  registry_->StopAll();

}



}  // namespace

}  // namespace bluetooth