make WorkRequests cancelable

8 files changed, 95 insertions, 64 deletions

diff --git a/src/mbgl/map/tile_data.cpp b/src/mbgl/map/tile_data.cpp
index 4025bacfcb..c6d87c7bde 100644
--- a/src/mbgl/map/tile_data.cpp
+++ b/src/mbgl/map/tile_data.cpp
@@ -4,6 +4,7 @@
 
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/util/worker.hpp>
+#include <mbgl/util/work_request.hpp>
 #include <mbgl/platform/log.hpp>
 
 using namespace mbgl;
@@ -56,9 +57,7 @@ void TileData::cancel() {
         env.cancelRequest(req);
         req = nullptr;
     }
-    if (workRequest) {
-        workRequest.join();
-    }
+    workRequest.reset();
 }
 
 bool TileData::mayStartParsing() {
diff --git a/src/mbgl/map/tile_data.hpp b/src/mbgl/map/tile_data.hpp
index 3be10dc034..ecc53a6b5e 100644
--- a/src/mbgl/map/tile_data.hpp
+++ b/src/mbgl/map/tile_data.hpp
@@ -7,7 +7,6 @@
 
 #include <mbgl/util/noncopyable.hpp>
 #include <mbgl/util/ptr.hpp>
-#include <mbgl/util/work_request.hpp>
 
 #include <atomic>
 #include <string>
@@ -21,6 +20,7 @@ class SourceInfo;
 class StyleLayer;
 class Request;
 class Worker;
+class WorkRequest;
 
 class TileData : private util::noncopyable {
 public:
@@ -79,7 +79,7 @@ protected:
     Request *req = nullptr;
     std::string data;
 
-    WorkRequest workRequest;
+    std::unique_ptr<WorkRequest> workRequest;
 
     // Contains the tile ID string for painting debug information.
     DebugFontBuffer debugFontBuffer;
diff --git a/src/mbgl/util/work_request.cpp b/src/mbgl/util/work_request.cpp
index a7d4b9c02e..69e9397288 100644
--- a/src/mbgl/util/work_request.cpp
+++ b/src/mbgl/util/work_request.cpp
@@ -1,38 +1,16 @@
 #include <mbgl/util/work_request.hpp>
+#include <mbgl/util/work_task.hpp>
 
-namespace mbgl {
-
-WorkRequest::WorkRequest() = default;
+#include <cassert>
 
-WorkRequest::WorkRequest(Future&& future, JoinedFlag flag)
-    : complete(std::move(future)),
-      joinedFlag(flag) {
-}
+namespace mbgl {
 
-WorkRequest::WorkRequest(WorkRequest&& o)
-    : complete(std::move(o.complete)),
-      joinedFlag(std::move(o.joinedFlag)) {
+WorkRequest::WorkRequest(Task task_) : task(task_) {
+    assert(task);
 }
 
 WorkRequest::~WorkRequest() {
-    if (complete.valid()) {
-        join();
-    }
-}
-
-WorkRequest& WorkRequest::operator=(WorkRequest&& o) {
-    complete = std::move(o.complete);
-    joinedFlag = std::move(o.joinedFlag);
-    return *this;
-}
-
-WorkRequest::operator bool() const {
-    return complete.valid();
-}
-
-void WorkRequest::join() {
-    *joinedFlag = true;
-    complete.get();
+    task->cancel();
 }
 
 }
diff --git a/src/mbgl/util/work_request.hpp b/src/mbgl/util/work_request.hpp
index f0f461b5d6..f2aa2bbacc 100644
--- a/src/mbgl/util/work_request.hpp
+++ b/src/mbgl/util/work_request.hpp
@@ -3,29 +3,20 @@
 
 #include <mbgl/util/noncopyable.hpp>
 
-#include <future>
+#include <memory>
 
 namespace mbgl {
 
+class WorkTask;
+
 class WorkRequest : public util::noncopyable {
 public:
-    using Future = std::future<void>;
-    using JoinedFlag = std::shared_ptr<std::atomic<bool>>;
-
-    WorkRequest();
-    WorkRequest(Future&&, JoinedFlag);
-    WorkRequest(WorkRequest&&);
+    using Task = std::shared_ptr<WorkTask>;
+    WorkRequest(Task);
     ~WorkRequest();
 
-    WorkRequest& operator=(WorkRequest&&);
-    operator bool() const;
-
-    // Wait for the worker task to complete.
-    void join();
-
 private:
-    Future complete;
-    JoinedFlag joinedFlag;
+    std::shared_ptr<WorkTask> task;
 };
 
 }
diff --git a/src/mbgl/util/work_task.cpp b/src/mbgl/util/work_task.cpp
new file mode 100644
index 0000000000..ebec420dec
--- /dev/null
+++ b/src/mbgl/util/work_task.cpp
@@ -0,0 +1,37 @@
+#include <mbgl/util/work_task.hpp>
+
+#include <cassert>
+
+namespace mbgl {
+
+WorkTask::WorkTask(std::function<void()> task_, std::function<void()> after_)
+    : task(task_), after(after_) {
+    assert(after);
+}
+
+void WorkTask::runTask() {
+    // We are only running the task when there's an after callback to call. This means that an
+    // empty after callback will be treated as a cancelled request. The mutex will be locked while
+    // processing so that the cancel() callback will block.
+    std::lock_guard<std::mutex> lock(mutex);
+    if (after) {
+        task();
+    }
+}
+
+void WorkTask::runAfter() {
+    if (after) {
+        after();
+    }
+}
+
+void WorkTask::cancel() {
+    // Remove the after callback to indicate that this callback has been canceled. The mutex will
+    // block when the task is currently in progres. When the task has not begun yet, the runTask()
+    // method will not do anything. When the task has been completed already, and the after callback
+    // was run as well, this will also do nothing.
+    std::lock_guard<std::mutex> lock(mutex);
+    after = {};
+}
+
+} // end namespace mbgl
diff --git a/src/mbgl/util/work_task.hpp b/src/mbgl/util/work_task.hpp
new file mode 100644
index 0000000000..f730a31c56
--- /dev/null
+++ b/src/mbgl/util/work_task.hpp
@@ -0,0 +1,27 @@
+#ifndef MBGL_UTIL_WORK_TASK
+#define MBGL_UTIL_WORK_TASK
+
+#include <mbgl/util/noncopyable.hpp>
+
+#include <functional>
+#include <mutex>
+
+namespace mbgl {
+
+class WorkTask : private util::noncopyable {
+public:
+    WorkTask(std::function<void()> task, std::function<void()> after);
+
+    void runTask();
+    void runAfter();
+    void cancel();
+
+private:
+    const std::function<void()> task;
+    std::function<void()> after;
+    std::mutex mutex;
+};
+
+} // end namespace mbgl
+
+#endif
diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp
index ad26ea5b18..984c55c95e 100644
--- a/src/mbgl/util/worker.cpp
+++ b/src/mbgl/util/worker.cpp
@@ -1,4 +1,6 @@
 #include <mbgl/util/worker.hpp>
+#include <mbgl/util/work_task.hpp>
+#include <mbgl/util/work_request.hpp>
 #include <mbgl/platform/platform.hpp>
 
 #include <cassert>
@@ -10,8 +12,8 @@ class Worker::Impl {
 public:
     Impl(uv_loop_t*) {}
 
-    void doWork(std::packaged_task<void ()>& task) {
-        task();
+    void doWork(std::shared_ptr<WorkTask>& task) {
+        task->runTask();
     }
 };
 
@@ -23,21 +25,16 @@ Worker::Worker(std::size_t count) {
 
 Worker::~Worker() = default;
 
-WorkRequest Worker::send(Fn work, Fn after) {
-    std::packaged_task<void ()> task(work);
-    std::future<void> future = task.get_future();
+std::unique_ptr<WorkRequest> Worker::send(Fn work, Fn after) {
+    auto task = std::make_shared<WorkTask>(work, after);
+    auto request = util::make_unique<WorkRequest>(task);
 
-    std::shared_ptr<std::atomic<bool>> joined = std::make_shared<std::atomic<bool>>();
-    *joined = false;
-
-    threads[current]->invokeWithResult(&Worker::Impl::doWork, [joined, after] {
-        if (!*joined) {
-            after();
-        }
+    threads[current]->invokeWithResult(&Worker::Impl::doWork, [task] {
+        task->runAfter();
     }, task);
 
     current = (current + 1) % threads.size();
-    return WorkRequest(std::move(future), joined);
+    return request;
 }
 
-}
+} // end namespace mbgl
diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp
index 461ba36e57..0c001e419b 100644
--- a/src/mbgl/util/worker.hpp
+++ b/src/mbgl/util/worker.hpp
@@ -2,13 +2,15 @@
 #define MBGL_UTIL_WORKER
 
 #include <mbgl/util/noncopyable.hpp>
-#include <mbgl/util/work_request.hpp>
 #include <mbgl/util/thread.hpp>
 
 #include <functional>
+#include <memory>
 
 namespace mbgl {
 
+class WorkRequest;
+
 class Worker : public mbgl::util::noncopyable {
 public:
     using Fn = std::function<void ()>;
@@ -26,7 +28,7 @@ public:
     // Together, this means that an object may make a work request with lambdas which
     // bind references to itself, and if and when those lambdas execute, the references
     // will still be valid.
-    WorkRequest send(Fn work, Fn after);
+    std::unique_ptr<WorkRequest> send(Fn work, Fn after);
 
 private:
     class Impl;