Restructure *Request and Context

The previous implementation, based on thread-local storage, did not
ensure that the context was destructed before the FileSource run loop.
This resulted in implementations attempting to uv_close handles for a
loop that had already been destroyed.

This change also fixes #1262.

9 files changed, 83 insertions, 192 deletions

diff --git a/src/mbgl/storage/asset_context.hpp b/src/mbgl/storage/asset_context.hpp
new file mode 100644
index 0000000000..44ebdba4ff
--- /dev/null
+++ b/src/mbgl/storage/asset_context.hpp
@@ -0,0 +1,23 @@
+#ifndef MBGL_STORAGE_DEFAULT_ASSET_CONTEXT
+#define MBGL_STORAGE_DEFAULT_ASSET_CONTEXT
+
+#include <mbgl/storage/request_base.hpp>
+
+typedef struct uv_loop_s uv_loop_t;
+
+namespace mbgl {
+
+class AssetContext {
+public:
+    static std::unique_ptr<AssetContext> createContext(uv_loop_t*);
+
+    virtual ~AssetContext() = default;
+    virtual RequestBase* createRequest(const Resource&,
+                                       RequestBase::Callback,
+                                       uv_loop_t*,
+                                       const std::string& assetRoot) = 0;
+};
+
+}
+
+#endif
diff --git a/src/mbgl/storage/asset_request.hpp b/src/mbgl/storage/asset_request.hpp
deleted file mode 100644
index 3f9b018b54..0000000000
--- a/src/mbgl/storage/asset_request.hpp
+++ /dev/null
@@ -1,27 +0,0 @@
-#ifndef MBGL_STORAGE_DEFAULT_ASSET_REQUEST
-#define MBGL_STORAGE_DEFAULT_ASSET_REQUEST
-
-#include "request_base.hpp"
-
-typedef struct uv_loop_s uv_loop_t;
-
-namespace mbgl {
-
-class AssetRequestImpl;
-
-class AssetRequest : public RequestBase {
-public:
-    AssetRequest(const Resource&, Callback, uv_loop_t*, const std::string& assetRoot);
-
-    void cancel() override;
-
-private:
-    ~AssetRequest();
-
-    AssetRequestImpl* impl;
-    friend class AssetRequestImpl;
-};
-
-}
-
-#endif
diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp
index b652e5e716..7ddfa08d93 100644
--- a/src/mbgl/storage/default_file_source.cpp
+++ b/src/mbgl/storage/default_file_source.cpp
@@ -1,7 +1,7 @@
 #include <mbgl/storage/default_file_source_impl.hpp>
 #include <mbgl/storage/request.hpp>
-#include <mbgl/storage/asset_request.hpp>
-#include <mbgl/storage/http_request.hpp>
+#include <mbgl/storage/asset_context.hpp>
+#include <mbgl/storage/http_context.hpp>
 
 #include <mbgl/storage/response.hpp>
 #include <mbgl/platform/platform.hpp>
@@ -64,7 +64,9 @@ void DefaultFileSource::cancel(Request *req) {
 DefaultFileSource::Impl::Impl(uv_loop_t* loop_, FileCache* cache_, const std::string& root)
     : loop(loop_),
       cache(cache_),
-      assetRoot(root.empty() ? platform::assetRoot() : root) {
+      assetRoot(root.empty() ? platform::assetRoot() : root),
+      assetContext(AssetContext::createContext(loop_)),
+      httpContext(HTTPContext::createContext(loop_)) {
 }
 
 DefaultFileRequest* DefaultFileSource::Impl::find(const Resource& resource) {
@@ -130,9 +132,9 @@ void DefaultFileSource::Impl::startRealRequest(const Resource& resource, std::sh
     };
 
     if (algo::starts_with(resource.url, "asset://")) {
-        request->request = new AssetRequest(resource, callback, loop, assetRoot);
+        request->request = assetContext->createRequest(resource, callback, loop, assetRoot);
     } else {
-        request->request = new HTTPRequest(resource, callback, loop, response);
+        request->request = httpContext->createRequest(resource, callback, loop, response);
     }
 }
 
diff --git a/src/mbgl/storage/default_file_source_impl.hpp b/src/mbgl/storage/default_file_source_impl.hpp
index 1d42f474ff..ed2d248d0a 100644
--- a/src/mbgl/storage/default_file_source_impl.hpp
+++ b/src/mbgl/storage/default_file_source_impl.hpp
@@ -2,6 +2,8 @@
 #define MBGL_STORAGE_DEFAULT_DEFAULT_FILE_SOURCE_IMPL
 
 #include <mbgl/storage/default_file_source.hpp>
+#include <mbgl/storage/asset_context.hpp>
+#include <mbgl/storage/http_context.hpp>
 
 #include <set>
 #include <unordered_map>
@@ -39,6 +41,8 @@ private:
     uv_loop_t* loop = nullptr;
     FileCache* cache = nullptr;
     const std::string assetRoot;
+    std::unique_ptr<AssetContext> assetContext;
+    std::unique_ptr<HTTPContext> httpContext;
 };
 
 }
diff --git a/src/mbgl/storage/http_context.cpp b/src/mbgl/storage/http_context.cpp
new file mode 100644
index 0000000000..c747490804
--- /dev/null
+++ b/src/mbgl/storage/http_context.cpp
@@ -0,0 +1,30 @@
+#include <mbgl/storage/http_context.hpp>
+
+namespace mbgl {
+
+HTTPContext::HTTPContext(uv_loop_t* loop_)
+    : reachability(loop_, [this] { retryRequests(); }) {
+    NetworkStatus::Subscribe(reachability.get());
+    reachability.unref();
+}
+
+HTTPContext::~HTTPContext() {
+    assert(requests.empty());
+    NetworkStatus::Unsubscribe(reachability.get());
+}
+
+void HTTPContext::addRequest(RequestBase* request) {
+    requests.insert(request);
+}
+
+void HTTPContext::removeRequest(RequestBase* request) {
+    requests.erase(request);
+}
+
+void HTTPContext::retryRequests() {
+    for (auto request : requests) {
+        request->retry();
+    }
+}
+
+}
diff --git a/src/mbgl/storage/http_context.hpp b/src/mbgl/storage/http_context.hpp
index 6b9518dab3..64a8afa6dd 100644
--- a/src/mbgl/storage/http_context.hpp
+++ b/src/mbgl/storage/http_context.hpp
@@ -1,76 +1,40 @@
 #ifndef MBGL_STORAGE_DEFAULT_HTTP_CONTEXT
 #define MBGL_STORAGE_DEFAULT_HTTP_CONTEXT
 
-#include "thread_context.hpp"
+#include <mbgl/storage/request_base.hpp>
 #include <mbgl/storage/network_status.hpp>
+#include <mbgl/util/uv_detail.hpp>
 
 #include <set>
 
 namespace mbgl {
 
-class HTTPRequest;
+class HTTPContext {
+public:
+    static std::unique_ptr<HTTPContext> createContext(uv_loop_t*);
 
-// This is a template class that provides a per-thread Context object. It can be used by HTTP
-// implementations to store global state. It also implements the NetworkStatus mechanism and
-// triggers immediate retries on all requests waiting for network status changes.
+    HTTPContext(uv_loop_t*);
+    virtual ~HTTPContext();
 
-template <typename Context>
-class HTTPContext : public ThreadContext<Context> {
-public:
-    HTTPContext(uv_loop_t *loop);
-    ~HTTPContext();
+    virtual RequestBase* createRequest(const Resource&,
+                                       RequestBase::Callback,
+                                       uv_loop_t*,
+                                       std::shared_ptr<const Response>) = 0;
 
-    void addRequest(HTTPRequest *request);
-    void removeRequest(HTTPRequest *baton);
+    void addRequest(RequestBase*);
+    void removeRequest(RequestBase*);
+
+private:
+    void retryRequests();
 
-public:
     // Will be fired when the network status becomes reachable.
-    uv_async_t *reachability = nullptr;
+    uv::async reachability;
 
     // A list of all pending HTTPRequestImpls that we need to notify when the network status
     // changes.
-    std::set<HTTPRequest *> requests;
+    std::set<RequestBase*> requests;
 };
 
-template <typename Context>
-HTTPContext<Context>::HTTPContext(uv_loop_t *loop_)
-    : ThreadContext<Context>(loop_) {
-    reachability = new uv_async_t;
-    reachability->data = this;
-#if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10
-    uv_async_init(loop_, reachability, [](uv_async_t *async, int) {
-#else
-    uv_async_init(loop_, reachability, [](uv_async_t *async) {
-#endif
-        for (auto request : reinterpret_cast<Context *>(async->data)->requests) {
-            request->retryImmediately();
-        }
-    });
-    // Allow the loop to quit even though this handle is still active.
-    uv_unref(reinterpret_cast<uv_handle_t *>(reachability));
-    NetworkStatus::Subscribe(reachability);
-}
-
-template <typename Context>
-HTTPContext<Context>::~HTTPContext() {
-    MBGL_VERIFY_THREAD(HTTPContext<Context>::tid);
-
-    assert(requests.empty());
-
-    NetworkStatus::Unsubscribe(reachability);
-    uv::close(reachability);
-}
-
-template <typename Context>
-void HTTPContext<Context>::addRequest(HTTPRequest *request) {
-    requests.insert(request);
-}
-
-template <typename Context>
-void HTTPContext<Context>::removeRequest(HTTPRequest *request) {
-    requests.erase(request);
-}
-
 }
 
 #endif
diff --git a/src/mbgl/storage/http_request.hpp b/src/mbgl/storage/http_request.hpp
deleted file mode 100644
index bbe1265c6d..0000000000
--- a/src/mbgl/storage/http_request.hpp
+++ /dev/null
@@ -1,28 +0,0 @@
-#ifndef MBGL_STORAGE_DEFAULT_HTTP_REQUEST
-#define MBGL_STORAGE_DEFAULT_HTTP_REQUEST
-
-#include "request_base.hpp"
-
-typedef struct uv_loop_s uv_loop_t;
-
-namespace mbgl {
-
-class HTTPRequestImpl;
-
-class HTTPRequest : public RequestBase {
-public:
-    HTTPRequest(const Resource&, Callback, uv_loop_t*, std::shared_ptr<const Response> = nullptr);
-
-    void cancel() override;
-    void retryImmediately();
-
-private:
-    ~HTTPRequest();
-
-    HTTPRequestImpl* impl;
-    friend class HTTPRequestImpl;
-};
-
-}
-
-#endif
diff --git a/src/mbgl/storage/request_base.hpp b/src/mbgl/storage/request_base.hpp
index e4ea7117c1..77c1d94229 100644
--- a/src/mbgl/storage/request_base.hpp
+++ b/src/mbgl/storage/request_base.hpp
@@ -23,6 +23,7 @@ public:
 
     virtual ~RequestBase() = default;
     virtual void cancel() = 0;
+    virtual void retry() {};
 
 protected:
     const Resource& resource;
diff --git a/src/mbgl/storage/thread_context.hpp b/src/mbgl/storage/thread_context.hpp
deleted file mode 100644
index 763c83a25b..0000000000
--- a/src/mbgl/storage/thread_context.hpp
+++ /dev/null
@@ -1,78 +0,0 @@
-#ifndef MBGL_STORAGE_DEFAULT_THREAD_CONTEXT
-#define MBGL_STORAGE_DEFAULT_THREAD_CONTEXT
-
-#include <mbgl/util/noncopyable.hpp>
-#include <mbgl/util/std.hpp>
-#include <mbgl/util/util.hpp>
-#include <mbgl/util/uv.hpp>
-
-#include <uv.h>
-#include <pthread.h>
-
-#include <map>
-#include <cassert>
-
-namespace mbgl {
-
-// This is a template class that provides a per-thread and per-loop Context object. It can be used
-// by implementations to store global state.
-
-template <typename Context>
-class ThreadContext : private util::noncopyable {
-protected:
-    MBGL_STORE_THREAD(tid)
-    using Map = std::map<uv_loop_t *, std::unique_ptr<Context>>;
-
-public:
-    static Context *Get(uv_loop_t *loop);
-
-private:
-    static pthread_key_t key;
-    static pthread_once_t once;
-
-public:
-    ThreadContext(uv_loop_t *loop);
-    ~ThreadContext();
-
-public:
-    uv_loop_t *loop;
-};
-
-template <typename Context>
-Context *ThreadContext<Context>::Get(uv_loop_t *loop) {
-    pthread_once(&once, []() {
-        pthread_key_create(&key, [](void *ptr) {
-            assert(ptr);
-            delete reinterpret_cast<Map *>(ptr);
-        });
-    });
-    auto contexts = reinterpret_cast<Map *>(pthread_getspecific(key));
-    if (!contexts) {
-        contexts = new Map();
-        pthread_setspecific(key, contexts);
-    }
-
-    // Now find a ThreadContext that matches the requested loop.
-    auto it = contexts->find(loop);
-    if (it == contexts->end()) {
-        auto result = contexts->emplace(loop, util::make_unique<Context>(loop));
-        assert(result.second); // Make sure it was actually inserted.
-        return result.first->second.get();
-    } else {
-        return it->second.get();
-    }
-}
-
-template <typename Context>
-ThreadContext<Context>::ThreadContext(uv_loop_t *loop_) : loop(loop_) {
-}
-
-template <typename Context>
-ThreadContext<Context>::~ThreadContext() {
-    MBGL_VERIFY_THREAD(tid);
-}
-
-
-}
-
-#endif