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

Fix deadlock in http request on hitting tile limit #13858

Merged
merged 1 commit into from
Feb 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: looks wake() does not need locked mutex

}

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