From 24695f6c0274cac8ae17df3765e0cc54c5668047 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Fri, 1 May 2015 17:14:06 -0400
Subject: [PATCH] More bandaids to avoid #1309

Even though we only bind the shared_ptr in the after callback, the
worker thread was still copying the callback and therefore gaining
a shared reference. Here we try to eliminate the copies.
---
 platform/default/sqlite_cache.cpp |  2 +-
 src/mbgl/util/run_loop.hpp        | 14 +++++++-------
 src/mbgl/util/thread.hpp          |  8 ++++----
 src/mbgl/util/worker.cpp          |  4 ++--
 src/mbgl/util/worker.hpp          |  2 +-
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/platform/default/sqlite_cache.cpp b/platform/default/sqlite_cache.cpp
index 6ba21c28130..83d42d5a5cf 100644
--- a/platform/default/sqlite_cache.cpp
+++ b/platform/default/sqlite_cache.cpp
@@ -131,7 +131,7 @@ void SQLiteCache::get(const Resource &resource, Callback callback) {
     // Will try to load the URL from the SQLite database and call the callback when done.
     // Note that the callback is probably going to invoked from another thread, so the caller
     // must make sure that it can run in that thread.
-    thread->invokeWithResult(&Impl::get, callback, resource);
+    thread->invokeWithResult(&Impl::get, std::move(callback), resource);
 }
 
 std::unique_ptr<Response> SQLiteCache::Impl::get(const Resource &resource) {
diff --git a/src/mbgl/util/run_loop.hpp b/src/mbgl/util/run_loop.hpp
index 971ad2cac7e..450718ebdbb 100644
--- a/src/mbgl/util/run_loop.hpp
+++ b/src/mbgl/util/run_loop.hpp
@@ -29,7 +29,7 @@ class RunLoop : private util::noncopyable {
 
     // Invoke fn() in the runloop thread, then invoke callback(result) in the current thread.
     template <class Fn, class R>
-    void invokeWithResult(Fn&& fn, std::function<void (R)> callback) {
+    void invokeWithResult(Fn&& fn, std::function<void (R)>&& callback) {
         RunLoop* outer = current.get();
         assert(outer);
 
@@ -37,23 +37,23 @@ class RunLoop : private util::noncopyable {
             /*
                 With C++14, we could write:
 
-                outer->invoke([callback, result = std::move(fn())] () mutable {
-                    callback(std::move(result));
+                outer->invoke([cb = std::move(callback), result = std::move(fn())] () mutable {
+                    cb(std::move(result));
                 });
 
                 Instead we're using a workaround with std::bind
                 to obtain move-capturing semantics with C++11:
                   http://stackoverflow.com/a/12744730/52207
             */
-            outer->invoke(std::bind([callback] (R& result) {
-                callback(std::move(result));
-            }, std::move(fn())));
+            outer->invoke(std::bind([] (std::function<void (R)>& cb, R& result) {
+                cb(std::move(result));
+            }, std::move(callback), std::move(fn())));
         });
     }
 
     // Invoke fn() in the runloop thread, then invoke callback() in the current thread.
     template <class Fn>
-    void invokeWithResult(Fn&& fn, std::function<void ()> callback) {
+    void invokeWithResult(Fn&& fn, std::function<void ()>&& callback) {
         RunLoop* outer = current.get();
         assert(outer);
 
diff --git a/src/mbgl/util/thread.hpp b/src/mbgl/util/thread.hpp
index 415bda38d89..189182464c1 100644
--- a/src/mbgl/util/thread.hpp
+++ b/src/mbgl/util/thread.hpp
@@ -49,14 +49,14 @@ class Thread {
 
     // Invoke object->fn(args...) in the runloop thread, then invoke callback(result) in the current thread.
     template <typename Fn, class R, class... Args>
-    void invokeWithResult(Fn fn, std::function<void (R)> callback, Args&&... args) {
-        loop->invokeWithResult(std::bind(fn, object, args...), callback);
+    void invokeWithResult(Fn fn, std::function<void (R)>&& callback, Args&&... args) {
+        loop->invokeWithResult(std::bind(fn, object, args...), std::move(callback));
     }
 
     // Invoke object->fn(args...) in the runloop thread, then invoke callback() in the current thread.
     template <typename Fn, class... Args>
-    void invokeWithResult(Fn fn, std::function<void ()> callback, Args&&... args) {
-        loop->invokeWithResult(std::bind(fn, object, args...), callback);
+    void invokeWithResult(Fn fn, std::function<void ()>&& callback, Args&&... args) {
+        loop->invokeWithResult(std::bind(fn, object, args...), std::move(callback));
     }
 
     // Invoke object->fn(args...) in the runloop thread, and wait for the result.
diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp
index fcbe6f5df9d..1a6eaf4d96d 100644
--- a/src/mbgl/util/worker.cpp
+++ b/src/mbgl/util/worker.cpp
@@ -21,8 +21,8 @@ Worker::Worker(std::size_t count) {
 
 Worker::~Worker() = default;
 
-void Worker::send(Fn work, Fn after) {
-    threads[current]->invokeWithResult(&Worker::Impl::doWork, after, work);
+void Worker::send(Fn&& work, Fn&& after) {
+    threads[current]->invokeWithResult(&Worker::Impl::doWork, std::move(after), std::move(work));
     current = (current + 1) % threads.size();
 }
 
diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp
index b71b4fd5b35..d8fbf6df7d7 100644
--- a/src/mbgl/util/worker.hpp
+++ b/src/mbgl/util/worker.hpp
@@ -15,7 +15,7 @@ class Worker : public mbgl::util::noncopyable {
     Worker(std::size_t count);
     ~Worker();
 
-    void send(Fn work, Fn after);
+    void send(Fn&& work, Fn&& after);
 
 private:
     class Impl;