diff options
author | Konstantin Käfer <mail@kkaefer.com> | 2014-09-22 13:06:40 +0200 |
---|---|---|
committer | Konstantin Käfer <mail@kkaefer.com> | 2014-09-24 16:16:29 +0200 |
commit | 3f38c7650e6d76fe5e6fd6d8f2f3174af0c2e176 (patch) | |
tree | c97cc8e65062c2d4faefbe1f5d0c337f1e1874b4 | |
parent | a142fd5e3a9da2a24ee3effc87519f9e1466655c (diff) | |
download | qtlocation-mapboxgl-3f38c7650e6d76fe5e6fd6d8f2f3174af0c2e176.tar.gz |
add retry timers
-rw-r--r-- | include/mbgl/storage/base_request.hpp | 10 | ||||
-rw-r--r-- | include/mbgl/storage/file_request.hpp | 2 | ||||
-rw-r--r-- | include/mbgl/storage/http_request.hpp | 4 | ||||
-rw-r--r-- | include/mbgl/storage/http_request_baton.hpp | 64 | ||||
-rw-r--r-- | include/mbgl/storage/response.hpp | 2 | ||||
-rw-r--r-- | include/mbgl/util/uv.hpp | 6 | ||||
-rw-r--r-- | src/storage/base_request.cpp | 12 | ||||
-rw-r--r-- | src/storage/file_request.cpp | 9 | ||||
-rw-r--r-- | src/storage/file_source.cpp | 9 | ||||
-rw-r--r-- | src/storage/http_request.cpp | 37 | ||||
-rw-r--r-- | src/storage/http_request_baton.cpp | 127 | ||||
-rw-r--r-- | src/storage/http_request_baton_darwin.mm | 75 | ||||
-rw-r--r-- | src/util/uv.cpp | 12 |
13 files changed, 326 insertions, 43 deletions
diff --git a/include/mbgl/storage/base_request.hpp b/include/mbgl/storage/base_request.hpp index 832ecbc23d..e9e6bfcb46 100644 --- a/include/mbgl/storage/base_request.hpp +++ b/include/mbgl/storage/base_request.hpp @@ -31,14 +31,22 @@ public: Callback *add(Callback &&callback, const util::ptr<BaseRequest> &request); void remove(Callback *callback); + + // Must be called by subclasses when a valid Response object is available. It will notify + // all listeners. void notify(); + // This function is called when the request ought to be stopped. Any subclass must make sure this + // is also called in its destructor. Calling this function repeatedly must be safe. + // This function must call notify(). + virtual void cancel() = 0; + public: const unsigned long thread_id; const std::string path; std::unique_ptr<Response> response; -private: +protected: // This object may hold a shared_ptr to itself. It does this to prevent destruction of this object // while a request is in progress. util::ptr<BaseRequest> self; diff --git a/include/mbgl/storage/file_request.hpp b/include/mbgl/storage/file_request.hpp index c99237ad4b..2f883728ff 100644 --- a/include/mbgl/storage/file_request.hpp +++ b/include/mbgl/storage/file_request.hpp @@ -14,6 +14,8 @@ public: FileRequest(const std::string &path, uv_loop_t *loop); ~FileRequest(); + void cancel(); + private: FileRequestBaton *ptr = nullptr; diff --git a/include/mbgl/storage/http_request.hpp b/include/mbgl/storage/http_request.hpp index ef3210d48d..61aed954f0 100644 --- a/include/mbgl/storage/http_request.hpp +++ b/include/mbgl/storage/http_request.hpp @@ -22,6 +22,8 @@ public: HTTPRequest(ResourceType type, const std::string &path, uv_loop_t *loop, util::ptr<SQLiteStore> store); ~HTTPRequest(); + void cancel(); + private: void loadedCacheEntry(std::unique_ptr<Response> &&response); @@ -31,6 +33,8 @@ private: HTTPRequestBaton *http_baton = nullptr; util::ptr<SQLiteStore> store; const ResourceType type; + + friend struct HTTPRequestBaton; }; } diff --git a/include/mbgl/storage/http_request_baton.hpp b/include/mbgl/storage/http_request_baton.hpp index 5f06a68cd1..47e75a203b 100644 --- a/include/mbgl/storage/http_request_baton.hpp +++ b/include/mbgl/storage/http_request_baton.hpp @@ -6,21 +6,83 @@ #include <string> typedef struct uv_async_s uv_async_t; +typedef struct uv_timer_s uv_timer_t; namespace mbgl { class HTTPRequest; +enum class HTTPResponseType : int8_t { + // This error probably won't be resolved by retrying anytime soon. We are giving up. + PermanentError = -5, + + // This error might be resolved by waiting some time (e.g. server issues). + // We are going to do an exponential back-off and will try again in a few seconds. + TemporaryError = -4, + + // This error was caused by a temporary error and it is likely that it will be resolved + // immediately. We are going to try again right away. This is like the TemporaryError, except + // that we will not perform exponential back-off. + SingularError = -3, + + // This error might be resolved once the network reachability status changes. + // We are going to watch the network status for changes and will retry as soon as the operating + // system notifies us of a network status change. + ConnectionError = -2, + + // The request was canceled programatically. + Canceled = -1, + + // The request is still in progress. + Unknown = 0, + + // The request returned data successfully. We retrieved and decoded the data successfully. + Successful = 1, + + // The request confirmed that the data wasn't changed. We already have the data. + NotModified = 2, +}; + struct HTTPRequestBaton { + HTTPRequestBaton(); + ~HTTPRequestBaton(); + + + const unsigned long thread_id; HTTPRequest *request = nullptr; std::string path; uv_async_t *async = nullptr; + uv_timer_t *timer = nullptr; std::unique_ptr<Response> response; void *ptr = nullptr; - bool not_modified = false; + HTTPResponseType type = HTTPResponseType::Unknown; + uint8_t attempts = 0; + + // IMPLEMENT THESE 3 PLATFORM SPECIFIC FUNCTIONS: + // Begin the HTTP request. Platform-specific implementation. void start(); + + // This will be called so that the baton can release resources. It is expected that the ptr is + // null after this call. Platform-specific implementation. + void cleanup(); + + // This will be called to cancel the HTTP request (if possible). Platform-specific implementation. void cancel(); + + + + // Called when the request should be canceled. This will call cancel() in turn. + void stop(); + + // After calling reset(), it is safe to call start() again on this baton object. + void reset(); + + // Will call start() after the specified timeout. + void retry(uint64_t delay); + + // Will be + static void notify(uv_async_t *async); }; } diff --git a/include/mbgl/storage/response.hpp b/include/mbgl/storage/response.hpp index 4960173f9e..8125656d81 100644 --- a/include/mbgl/storage/response.hpp +++ b/include/mbgl/storage/response.hpp @@ -6,6 +6,8 @@ namespace mbgl { + + class Response { public: long code = 0; diff --git a/include/mbgl/util/uv.hpp b/include/mbgl/util/uv.hpp index 80fff9c96f..3c533cfbf8 100644 --- a/include/mbgl/util/uv.hpp +++ b/include/mbgl/util/uv.hpp @@ -4,6 +4,7 @@ #include <string> typedef struct uv_async_s uv_async_t; +typedef struct uv_timer_s uv_timer_t; typedef struct uv_handle_s uv_handle_t; typedef struct uv_loop_s uv_loop_t; @@ -11,6 +12,11 @@ namespace uv { std::string cwd(); +struct deleter { + void operator()(uv_async_t *async); + void operator()(uv_timer_t *timer); +}; + class thread; class rwlock; class loop; diff --git a/src/storage/base_request.cpp b/src/storage/base_request.cpp index 8eeda81794..fead001dad 100644 --- a/src/storage/base_request.cpp +++ b/src/storage/base_request.cpp @@ -25,15 +25,12 @@ BaseRequest::BaseRequest(const std::string &path_) : thread_id(uv_thread_self()) // notify all cancel callbacks. BaseRequest::~BaseRequest() { assert(thread_id == uv_thread_self()); - invoke<AbortedCallback>(callbacks); + notify(); } void BaseRequest::notify() { assert(thread_id == uv_thread_self()); - // We are only supposed to call notify when a result exists. - assert(response); - // The parameter exists solely so that any calls to ->remove() // are not going to cause deallocation of this object while this call is in progress. util::ptr<BaseRequest> retain = self; @@ -42,7 +39,12 @@ void BaseRequest::notify() { // on the request object, which would modify the list. const std::forward_list<std::unique_ptr<Callback>> list = std::move(callbacks); callbacks.clear(); - invoke<CompletedCallback>(list, *response); + + if (response) { + invoke<CompletedCallback>(list, *response); + } else { + invoke<AbortedCallback>(list); + } self.reset(); } diff --git a/src/storage/file_request.cpp b/src/storage/file_request.cpp index 3fe0db0c26..c9e9c1b61d 100644 --- a/src/storage/file_request.cpp +++ b/src/storage/file_request.cpp @@ -166,7 +166,7 @@ FileRequest::FileRequest(const std::string &path_, uv_loop_t *loop) : BaseRequest(path_), ptr(new FileRequestBaton(this, path, loop)) { } -FileRequest::~FileRequest() { +void FileRequest::cancel() { assert(thread_id == uv_thread_self()); if (ptr) { @@ -175,7 +175,14 @@ FileRequest::~FileRequest() { // When deleting a FileRequest object with a uv_fs_* call is in progress, we are making sure // that the callback doesn't accidentally reference this object again. ptr->request = nullptr; + ptr = nullptr; } + +} + +FileRequest::~FileRequest() { + assert(thread_id == uv_thread_self()); + cancel(); } } diff --git a/src/storage/file_source.cpp b/src/storage/file_source.cpp index 1f8b0dd8e2..ef2b036e50 100644 --- a/src/storage/file_source.cpp +++ b/src/storage/file_source.cpp @@ -25,6 +25,15 @@ FileSource::~FileSource() { uv_messenger_stop(queue); // NOTE: We don't need to delete the messenger since it will be deleted by the // uv_messenger_stop() function. + + util::ptr<BaseRequest> request; + + // Send a cancel() message to all requests that we are still holding. + for (const std::pair<std::string, std::weak_ptr<BaseRequest>> &pair : pending) { + if ((request = pair.second.lock())) { + request->cancel(); + } + } } void FileSource::setBase(const std::string &value) { diff --git a/src/storage/http_request.cpp b/src/storage/http_request.cpp index 3d456a836d..c0863a6ade 100644 --- a/src/storage/http_request.cpp +++ b/src/storage/http_request.cpp @@ -1,5 +1,4 @@ #include <mbgl/storage/http_request.hpp> -#include <mbgl/storage/request.hpp> #include <mbgl/storage/sqlite_store.hpp> #include <mbgl/storage/http_request_baton.hpp> @@ -18,6 +17,7 @@ namespace mbgl { struct CacheRequestBaton { HTTPRequest *request = nullptr; std::string path; + util::ptr<SQLiteStore> store; uv_loop_t *loop; }; @@ -26,6 +26,7 @@ HTTPRequest::HTTPRequest(ResourceType type_, const std::string &path, uv_loop_t cache_baton = new CacheRequestBaton; cache_baton->request = this; cache_baton->path = path; + cache_baton->store = store; cache_baton->loop = loop; store->get(path, [](std::unique_ptr<Response> &&response, void *ptr) { // Wrap in a unique_ptr, so it'll always get auto-destructed. @@ -68,28 +69,12 @@ void HTTPRequest::loadedCacheEntry(std::unique_ptr<Response> &&response) { http_baton->async = new uv_async_t; http_baton->response = std::move(response); http_baton->async->data = http_baton; - uv_async_init(cache_baton->loop, http_baton->async, [](uv_async_t *async) { - std::unique_ptr<HTTPRequestBaton> baton((HTTPRequestBaton *)async->data); - if (baton->request) { - assert(uv_thread_self() == baton->request->thread_id); - baton->request->response = std::move(baton->response); - if (baton->not_modified) { - baton->request->store->updateExpiration(baton->path, baton->request->response->expires); - } else { - baton->request->store->put(baton->path, baton->request->type, *baton->request->response); - } - baton->request->notify(); - // Note: after calling notify(), the baton object may cease to exist. - } - - uv_close((uv_handle_t *)async, [](uv_handle_t *handle) { - delete (uv_async_t *)handle; - }); - }); + uv_async_init(cache_baton->loop, http_baton->async, HTTPRequestBaton::notify); + http_baton->reset(); http_baton->start(); } -HTTPRequest::~HTTPRequest() { +void HTTPRequest::cancel() { assert(uv_thread_self() == thread_id); if (cache_baton) { @@ -99,12 +84,22 @@ HTTPRequest::~HTTPRequest() { // Note: We don't manually delete the CacheRequestBaton since it'll be deleted by the // callback. cache_baton->request = nullptr; + cache_baton = nullptr; } if (http_baton) { http_baton->request = nullptr; - http_baton->cancel(); + http_baton->stop(); + http_baton = nullptr; } + + notify(); +} + + +HTTPRequest::~HTTPRequest() { + assert(uv_thread_self() == thread_id); + cancel(); } #pragma clang diagnostic pop diff --git a/src/storage/http_request_baton.cpp b/src/storage/http_request_baton.cpp new file mode 100644 index 0000000000..7130662b2b --- /dev/null +++ b/src/storage/http_request_baton.cpp @@ -0,0 +1,127 @@ +#include <mbgl/storage/http_request_baton.hpp> +#include <mbgl/storage/http_request.hpp> +#include <mbgl/storage/sqlite_store.hpp> +#include <mbgl/util/uv.hpp> + +#include <uv.h> + +#include <cassert> + +namespace mbgl { + +HTTPRequestBaton::HTTPRequestBaton() : thread_id(uv_thread_self()) { +} + +HTTPRequestBaton::~HTTPRequestBaton() { + assert(uv_thread_self() == thread_id); + if (timer) { + timer->data = nullptr; + } +} + +void HTTPRequestBaton::reset() { + attempts++; + response.reset(); + type = HTTPResponseType::Unknown; +} + +void HTTPRequestBaton::retry(uint64_t delay) { + assert(uv_thread_self() == thread_id); + assert(!timer); + timer = new uv_timer_t; + timer->data = this; + uv_timer_init(async->loop, timer); + uv_timer_start(timer, [](uv_timer_t *timer) { + std::unique_ptr<uv_timer_t, uv::deleter> timer_guard(timer); + uv_timer_stop(timer); + if (timer->data) { + HTTPRequestBaton *baton = reinterpret_cast<HTTPRequestBaton *>(timer->data); + assert(uv_thread_self() == baton->thread_id); + baton->timer = nullptr; + baton->start(); + } + }, delay, 0); +} + +void HTTPRequestBaton::stop() { + if (timer) { + uv_timer_stop(timer); + uv::deleter()(timer); + timer = nullptr; + } + + cancel(); +} + + +void HTTPRequestBaton::notify(uv_async_t *async_unguarded) { + // Ensure deletion of pointers. + std::unique_ptr<uv_async_t, uv::deleter> async(async_unguarded); + std::unique_ptr<HTTPRequestBaton> baton((HTTPRequestBaton *)async->data); + baton->cleanup(); + + if (baton->request) { + assert(uv_thread_self() == baton->request->thread_id); + switch (baton->type) { + case HTTPResponseType::TemporaryError: + case HTTPResponseType::ConnectionError: + // Exponential back-off: + if (baton->attempts >= 4) { + // Fail immediately. + baton->request->response = std::move(baton->response); + baton->request->notify(); + } else { + baton->reset(); + baton->retry(1000 * (1 << baton->attempts)); + // Make sure we're not deleting the objects, since we're now reusing them. + baton.release(); + async.release(); + } + break; + + case HTTPResponseType::SingularError: + // Retry immediately. + if (baton->attempts >= 4) { + // Fail immediately. + baton->request->response = std::move(baton->response); + baton->request->notify(); + } else { + baton->reset(); + baton->start(); + // Make sure we're not deleting the objects, since we're now reusing them. + baton.release(); + async.release(); + } + break; + + case HTTPResponseType::Canceled: + baton->request->response.reset(); + baton->request->notify(); + break; + + case HTTPResponseType::PermanentError: + case HTTPResponseType::Successful: + baton->request->store->put(baton->path, baton->request->type, *baton->response); + baton->request->response = std::move(baton->response); + baton->request->notify(); + break; + + case HTTPResponseType::NotModified: + baton->request->store->updateExpiration(baton->path, baton->response->expires); + baton->request->response = std::move(baton->response); + baton->request->notify(); + break; + + default: + // This should never happen. + assert(false); + break; + } + // Note: after calling notify(), the baton object may cease to exist. + } else { + // The HTTPRequest object has already been destructed. There is nobody we need to notify + // anymore. All we need to do is to cleanup. + } +} + +} diff --git a/src/storage/http_request_baton_darwin.mm b/src/storage/http_request_baton_darwin.mm index b1e47e3508..eaf75ad4d1 100644 --- a/src/storage/http_request_baton_darwin.mm +++ b/src/storage/http_request_baton_darwin.mm @@ -1,8 +1,11 @@ #include <mbgl/storage/http_request_baton.hpp> +#include <mbgl/util/std.hpp> #include <mbgl/util/parsedate.h> #include <uv.h> +#include <mbgl/util/uv.hpp> + #import <Foundation/Foundation.h> #include <ctime> #include <xlocale.h> @@ -14,6 +17,7 @@ NSURLSession *session = nullptr; void HTTPRequestBaton::start() { // Starts the request. + assert(!ptr); // Create a C locale static locale_t locale = newlocale(LC_ALL_MASK, nullptr, nullptr); @@ -28,7 +32,6 @@ void HTTPRequestBaton::start() { // TODO: add a delegate to the session that prohibits caching, since we handle this ourselves. }); - NSMutableURLRequest *request = [[NSMutableURLRequest alloc] initWithURL:[NSURL URLWithString:@(path.c_str())]]; if (response && response->modified) { struct tm *timeinfo; @@ -41,30 +44,63 @@ void HTTPRequestBaton::start() { NSURLSessionDataTask *task = [session dataTaskWithRequest:request completionHandler:^(NSData *data, NSURLResponse *res, NSError *error) { - if (error) { - if ([error code] != NSURLErrorCancelled) { + if ([error code] == NSURLErrorCancelled) { + // The response code remains at 0 to indicate cancelation. + // In addition, we don't need any response object. + response.reset(); + type = HTTPResponseType::Canceled; + } else { // TODO: Use different codes for host not found, timeout, invalid URL etc. // These can be categorized in temporary and permanent errors. - response = std::unique_ptr<Response>(new Response); - response->code = -1; + response = std::make_unique<Response>(); + response->code = [(NSHTTPURLResponse *)res statusCode]; response->message = [[error localizedDescription] UTF8String]; - } else { - // This request was canceled. - // The response code remains at 0 to indicate cancelation. + + switch ([error code]) { + case NSURLErrorBadServerResponse: // 5xx errors + type = HTTPResponseType::TemporaryError; + break; + + case NSURLErrorTimedOut: + case NSURLErrorUserCancelledAuthentication: + type = HTTPResponseType::SingularError; // retry immediately + break; + + case NSURLErrorNetworkConnectionLost: + case NSURLErrorCannotFindHost: + case NSURLErrorCannotConnectToHost: + case NSURLErrorDNSLookupFailed: + case NSURLErrorNotConnectedToInternet: + case NSURLErrorInternationalRoamingOff: + case NSURLErrorCallIsActive: + case NSURLErrorDataNotAllowed: + type = HTTPResponseType::ConnectionError; + break; + + default: + type = HTTPResponseType::PermanentError; + } } } else if ([res isKindOfClass:[NSHTTPURLResponse class]]) { const long code = [(NSHTTPURLResponse *)res statusCode]; if (code == 304) { // Assume a Response object already exists. assert(response); - not_modified = true; } else { - response = std::unique_ptr<Response>(new Response); + response = std::make_unique<Response>(); response->code = code; response->data = {(const char *)[data bytes], [data length]}; } + if (code == 304) { + type = HTTPResponseType::NotModified; + } else if (code == 200) { + type = HTTPResponseType::Successful; + } else { + assert(!"code must be either 200 or 304"); + } + NSDictionary *headers = [(NSHTTPURLResponse *)res allHeaderFields]; NSString *cache_control = [headers objectForKey:@"Cache-Control"]; if (cache_control) { @@ -77,26 +113,37 @@ void HTTPRequestBaton::start() { } } else { // This should never happen. - response = std::unique_ptr<Response>(new Response); + type = HTTPResponseType::PermanentError; + response = std::make_unique<Response>(); response->code = -1; response->message = "response class is not NSHTTPURLResponse"; - // TODO: If we get a failure, we should not overwrite a potentially existing response. } uv_async_send(async); }]; + [task resume]; + ptr = const_cast<void *>(CFBridgingRetain(task)); +} - [task resume]; +void HTTPRequestBaton::cleanup() { + if (ptr) { + CFBridgingRelease(ptr); + ptr = nullptr; + } } void HTTPRequestBaton::cancel() { // After this function returns, the HTTPRequestBaton object may cease to exist at any time. // try to stop the request if (ptr) { - [(NSURLSessionDataTask *)CFBridgingRelease(ptr) cancel]; + NSURLSessionDataTask *task = CFBridgingRelease(ptr); ptr = nullptr; + [task cancel]; + } else { + // Currently, there is no request in progress. We can delete the async right away. + uv_async_send(async); } } diff --git a/src/util/uv.cpp b/src/util/uv.cpp index 65c790b9c0..b97074f054 100644 --- a/src/util/uv.cpp +++ b/src/util/uv.cpp @@ -16,4 +16,16 @@ std::string cwd() { return dir; } +void deleter::operator()(uv_async_t *async) { + uv_close((uv_handle_t *)async, [](uv_handle_t *handle) { + delete (uv_async_t *)handle; + }); +} + +void deleter::operator()(uv_timer_t *timer) { + uv_close((uv_handle_t *)timer, [](uv_handle_t *handle) { + delete (uv_timer_t *)handle; + }); +} + } |