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

Commit 0aaf639b authored by Myles Watson's avatar Myles Watson Committed by android-build-merger
Browse files

OS: Add ordering guarantees for Handler.Clear() am: 3bd06b7f am: 5077cc45 

am: bc37b683

Change-Id: Ie8d9bb4278fe1445568e6901a6df9b2f65abcffa
parents 535edbd9 bc37b683

system/gd/common/bidi_queue_unittest.cc

+1 −0
Original line number Original line Diff line number Diff line
@@ -64,6 +64,7 @@ class TestBidiQueueEnd { 
      : handler_(handler), end_(end) {}

      : handler_(handler), end_(end) {}





  ~TestBidiQueueEnd() {

  ~TestBidiQueueEnd() {

    handler_->Clear();

  }

  }





  std::promise<void>* Send(TA* value) {

  std::promise<void>* Send(TA* value) {

system/gd/module.cc

+4 −2
Original line number Original line Diff line number Diff line
@@ -83,12 +83,14 @@ Module* ModuleRegistry::Start(const ModuleFactory* module, Thread* thread) { 
}

}





void ModuleRegistry::StopAll() {

void ModuleRegistry::StopAll() {

  // Since modules were brought up in dependency order,

  // Since modules were brought up in dependency order, it is safe to tear down by going in reverse order.

  // it is safe to tear down by going in reverse order.

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

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

    auto instance = started_modules_.find(*it);

    auto instance = started_modules_.find(*it);

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

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





    // Clear the handler before stopping the module to allow it to shut down gracefully.

    instance->second->handler_->Clear();

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

    instance->second->Stop();

    instance->second->Stop();





    delete instance->second->handler_;

    delete instance->second->handler_;

system/gd/os/handler.h

+7 −1
Original line number Original line Diff line number Diff line
@@ -46,6 +46,9 @@ class Handler { 
  // Remove all pending events from the queue of this handler

  // Remove all pending events from the queue of this handler

  void Clear();

  void Clear();





  // Die if the current reactable doesn't stop before the timeout.  Must be called after Clear()

  void WaitUntilStopped(std::chrono::milliseconds timeout);



  template <typename T>

  template <typename T>

  friend class Queue;

  friend class Queue;
@@ -54,7 +57,10 @@ class Handler { 
  friend class RepeatingAlarm;

  friend class RepeatingAlarm;





 private:

 private:

  std::queue<Closure> tasks_;

  inline bool was_cleared() const {

    return tasks_ == nullptr;

  };

  std::queue<Closure>* tasks_;

  Thread* thread_;

  Thread* thread_;

  int fd_;

  int fd_;

  Reactor::Reactable* reactable_;

  Reactor::Reactable* reactable_;

system/gd/os/linux_generic/alarm_unittest.cc

+1 −0
Original line number Original line Diff line number Diff line
@@ -34,6 +34,7 @@ class AlarmTest : public ::testing::Test { 




  void TearDown() override {

  void TearDown() override {

    delete alarm_;

    delete alarm_;

    handler_->Clear();

    delete handler_;

    delete handler_;

    delete thread_;

    delete thread_;

  }

  }

system/gd/os/linux_generic/handler.cc

+36 −22
Original line number Original line Diff line number Diff line
@@ -17,8 +17,8 @@ 
#include "os/handler.h"

#include "os/handler.h"





#include <sys/eventfd.h>

#include <sys/eventfd.h>

#include <cstring>

#include <unistd.h>

#include <unistd.h>

#include <cstring>





#include "os/log.h"

#include "os/log.h"

#include "os/reactor.h"

#include "os/reactor.h"
@@ -31,15 +31,17 @@ 
namespace bluetooth {

namespace bluetooth {

namespace os {

namespace os {





Handler::Handler(Thread* thread) : thread_(thread), fd_(eventfd(0, EFD_SEMAPHORE | EFD_NONBLOCK)) {

Handler::Handler(Thread* thread)

    : tasks_(new std::queue<Closure>()), thread_(thread), fd_(eventfd(0, EFD_SEMAPHORE | EFD_NONBLOCK)) {

  ASSERT(fd_ != -1);

  ASSERT(fd_ != -1);



  reactable_ = thread_->GetReactor()->Register(fd_, [this] { this->handle_next_event(); }, nullptr);

  reactable_ = thread_->GetReactor()->Register(fd_, [this] { this->handle_next_event(); }, nullptr);

}

}





Handler::~Handler() {

Handler::~Handler() {

  thread_->GetReactor()->Unregister(reactable_);

  {

  reactable_ = nullptr;

    std::lock_guard<std::mutex> lock(mutex_);

    ASSERT_LOG(was_cleared(), "Handlers must be cleared before they are destroyed");

  }





  int close_status;

  int close_status;

  RUN_NO_INTR(close_status = close(fd_));

  RUN_NO_INTR(close_status = close(fd_));
@@ -49,7 +51,10 @@ Handler::~Handler() { 
void Handler::Post(Closure closure) {

void Handler::Post(Closure closure) {

  {

  {

    std::lock_guard<std::mutex> lock(mutex_);

    std::lock_guard<std::mutex> lock(mutex_);

    tasks_.emplace(std::move(closure));

    if (was_cleared()) {

      return;

    }

    tasks_->emplace(std::move(closure));

  }

  }

  uint64_t val = 1;

  uint64_t val = 1;

  auto write_result = eventfd_write(fd_, val);

  auto write_result = eventfd_write(fd_, val);
@@ -57,32 +62,41 @@ void Handler::Post(Closure closure) { 
}

}





void Handler::Clear() {

void Handler::Clear() {

  std::queue<Closure>* tmp = nullptr;

  {

    std::lock_guard<std::mutex> lock(mutex_);

    std::lock_guard<std::mutex> lock(mutex_);



    ASSERT_LOG(!was_cleared(), "Handlers must only be cleared once");

  std::queue<Closure> empty;

    std::swap(tasks_, tmp);

  std::swap(tasks_, empty);

  }

  delete tmp;





  uint64_t val;

  uint64_t val;

  while (eventfd_read(fd_, &val) == 0) {

  while (eventfd_read(fd_, &val) == 0) {

  }

  }



  thread_->GetReactor()->Unregister(reactable_);

  reactable_ = nullptr;

}



void Handler::WaitUntilStopped(std::chrono::milliseconds timeout) {

  ASSERT(reactable_ == nullptr);

  ASSERT(thread_->GetReactor()->WaitForUnregisteredReactable(timeout));

}

}





void Handler::handle_next_event() {

void Handler::handle_next_event() {

  Closure closure;

  Closure closure;

  {

    std::lock_guard<std::mutex> lock(mutex_);

    uint64_t val = 0;

    uint64_t val = 0;

    auto read_result = eventfd_read(fd_, &val);

    auto read_result = eventfd_read(fd_, &val);

  if (read_result == -1 && errno == EAGAIN) {



    // We were told there was an item, but it was removed before we got there

    if (was_cleared()) {

    // (aka the queue was cleared). Not a fatal error, so just bail.

      return;

      return;

    }

    }

    ASSERT_LOG(read_result != -1, "eventfd read error %d %s", errno, strerror(errno));





  ASSERT(read_result != -1);

    closure = std::move(tasks_->front());



    tasks_->pop();

  {

    std::lock_guard<std::mutex> lock(mutex_);

    closure = std::move(tasks_.front());

    tasks_.pop();

  }

  }

  closure();

  closure();

}

}