Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[android] change lock scope in run_loop to avoid deadlocks. simplify as
Browse files Browse the repository at this point in the history
well
  • Loading branch information
ivovandongen committed Feb 12, 2019
1 parent ca7d942 commit 1fadf56
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 48 deletions.
1 change: 0 additions & 1 deletion platform/android/src/async_task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class AsyncTask::Impl : public RunLoop::Impl::Runnable {
public:
Impl(std::function<void()>&& fn)
: queued(true), task(std::move(fn)) {
loop->initRunnable(this);
}

~Impl() {
Expand Down
62 changes: 27 additions & 35 deletions platform/android/src/run_loop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,55 +141,47 @@ void RunLoop::Impl::wake() {
}

void RunLoop::Impl::addRunnable(Runnable* runnable) {
std::lock_guard<std::recursive_mutex> lock(mtx);

if (runnable->iter == runnables.end()) {
auto iter = runnables.insert(runnables.end(), runnable);
runnable->iter = std::move(iter);
{
std::lock_guard<std::mutex> lock(mutex);
runnables.push_back(runnable);
}

wake();
}

void RunLoop::Impl::removeRunnable(Runnable* runnable) {
std::lock_guard<std::recursive_mutex> lock(mtx);

if (runnable->iter == runnables.end()) {
return;
}

if (nextRunnable == runnable->iter) {
++nextRunnable;
}

runnables.erase(runnable->iter);
runnable->iter = runnables.end();
}

void RunLoop::Impl::initRunnable(Runnable* runnable) {
std::lock_guard<std::recursive_mutex> lock(mtx);
runnable->iter = runnables.end();
std::lock_guard<std::mutex> lock(mutex);
runnables.remove(runnable);
}

Milliseconds RunLoop::Impl::processRunnables() {
std::lock_guard<std::recursive_mutex> lock(mtx);

auto now = Clock::now();
auto nextDue = TimePoint::max();

// O(N) but in the render thread where we get tons
// of messages, the size of the list is usually 1~2.
for (nextRunnable = runnables.begin(); nextRunnable != runnables.end();) {
Runnable* runnable = *(nextRunnable++);

auto const dueTime = runnable->dueTime();
if (dueTime <= now) {
runnable->runTask();
} else {
nextDue = std::min(nextDue, dueTime);
std::list<Runnable*> tmp;
{
std::lock_guard<std::mutex> lock(mutex);

// O(N) but in the render thread where we get tons
// of messages, the size of the list is usually 1~2.
auto it = runnables.begin();
while (it != runnables.end()) {
Runnable* runnable = *it;

auto const dueTime = runnable->dueTime();
if (dueTime <= now) {
tmp.push_back(runnable);
runnables.erase(it++);
} else {
nextDue = std::min(nextDue, dueTime);
++it;
}
}
}

for (auto runnable : tmp) {
runnable->runTask();
}

if (runnables.empty() || nextDue == TimePoint::max()) {
return Milliseconds(-1);
}
Expand Down
6 changes: 1 addition & 5 deletions platform/android/src/run_loop_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ class RunLoop::Impl {

virtual void runTask() = 0;
virtual TimePoint dueTime() const = 0;

std::list<Runnable*>::iterator iter;
};

Impl(RunLoop*, RunLoop::Type);
Expand All @@ -37,7 +35,6 @@ class RunLoop::Impl {

void addRunnable(Runnable*);
void removeRunnable(Runnable*);
void initRunnable(Runnable*);

Milliseconds processRunnables();

Expand All @@ -55,9 +52,8 @@ class RunLoop::Impl {

std::unique_ptr<Thread<Alarm>> alarm;

std::recursive_mutex mtx;
std::mutex mutex;
std::list<Runnable*> runnables;
std::list<Runnable*>::iterator nextRunnable = runnables.end();
};

} // namespace util
Expand Down
12 changes: 5 additions & 7 deletions platform/android/src/timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,20 @@ namespace util {

class Timer::Impl : public RunLoop::Impl::Runnable {
public:
Impl() {
loop->initRunnable(this);
}
Impl() = default;

~Impl() {
stop();
}

void start(Duration timeout, Duration repeat_, std::function<void ()>&& task_) {
void start(Duration timeout, Duration repeat_, std::function<void()>&& task_) {
stop();

repeat = repeat_;
task = std::move(task_);
// Prevent overflows when timeout is set to Duration::max()
due = (timeout == Duration::max()) ? std::chrono::time_point<Clock>::max() : Clock::now() + timeout;
due = (timeout == Duration::max()) ? std::chrono::time_point<Clock>::max() : Clock::now() +
timeout;
loop->addRunnable(this);
}

Expand Down Expand Up @@ -59,8 +58,7 @@ class Timer::Impl : public RunLoop::Impl::Runnable {
std::function<void()> task;
};

Timer::Timer()
: impl(std::make_unique<Impl>()) {
Timer::Timer() : impl(std::make_unique<Impl>()) {
}

Timer::~Timer() = default;
Expand Down

0 comments on commit 1fadf56

Please sign in to comment.