diff options
author | John Firebaugh <john.firebaugh@gmail.com> | 2015-04-28 14:31:00 -0400 |
---|---|---|
committer | John Firebaugh <john.firebaugh@gmail.com> | 2015-04-28 19:26:35 -0400 |
commit | 481fa0e69ff8d84202e91a1643c72636200f1b80 (patch) | |
tree | a2679fd559046d2c7e6f64cb5c4a67e29e16bf66 /src | |
parent | 2c3952af66d6907f8a892d42d0034e91126b073a (diff) | |
download | qtlocation-mapboxgl-481fa0e69ff8d84202e91a1643c72636200f1b80.tar.gz |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/mbgl/storage/asset_context.hpp | 23 | ||||
-rw-r--r-- | src/mbgl/storage/asset_request.hpp | 27 | ||||
-rw-r--r-- | src/mbgl/storage/default_file_source.cpp | 12 | ||||
-rw-r--r-- | src/mbgl/storage/default_file_source_impl.hpp | 4 | ||||
-rw-r--r-- | src/mbgl/storage/http_context.cpp | 30 | ||||
-rw-r--r-- | src/mbgl/storage/http_context.hpp | 72 | ||||
-rw-r--r-- | src/mbgl/storage/http_request.hpp | 28 | ||||
-rw-r--r-- | src/mbgl/storage/request_base.hpp | 1 | ||||
-rw-r--r-- | src/mbgl/storage/thread_context.hpp | 78 | ||||
-rw-r--r-- | src/mbgl/util/uv_detail.hpp | 4 |
10 files changed, 87 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 diff --git a/src/mbgl/util/uv_detail.hpp b/src/mbgl/util/uv_detail.hpp index 96d5442462..fbc0ec5aa0 100644 --- a/src/mbgl/util/uv_detail.hpp +++ b/src/mbgl/util/uv_detail.hpp @@ -104,6 +104,10 @@ public: uv_unref(reinterpret_cast<uv_handle_t*>(a.get())); } + inline uv_async_t* get() { + return a.get(); + } + private: #if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10 static void async_cb(uv_async_t* a, int) { |