diff --git a/include/mbgl/storage/default/request.hpp b/include/mbgl/storage/default/request.hpp index b686d1fe9..00157329b 100644 --- a/include/mbgl/storage/default/request.hpp +++ b/include/mbgl/storage/default/request.hpp @@ -6,6 +6,8 @@ #include #include +#include +#include #include #include @@ -35,12 +37,12 @@ public: private: ~Request(); void invoke(); - static void notifyCallback(uv_async_t *async); - static void cancelCallback(uv_async_t *async); + void notifyCallback(); private: - uv_async_t *notifyAsync = nullptr; - uv_async_t *destructAsync = nullptr; + uv_async_t *async = nullptr; + struct Canceled; + std::unique_ptr canceled; Callback callback; std::shared_ptr response; diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp index 48b5b774d..ed7f625e8 100644 --- a/src/mbgl/storage/request.cpp +++ b/src/mbgl/storage/request.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -12,39 +13,39 @@ namespace mbgl { +struct Request::Canceled { std::mutex mutex; bool confirmed = false; }; + // Note: This requires that loop is running in the current thread (or not yet running). Request::Request(const Resource &resource_, uv_loop_t *loop, const Environment &env_, Callback callback_) : callback(callback_), resource(resource_), env(env_) { // When there is no loop supplied (== nullptr), the callback will be fired in an arbitrary // thread (the thread notify() is called from) rather than kicking back to the calling thread. if (loop) { - notifyAsync = new uv_async_t; - notifyAsync->data = nullptr; + async = new uv_async_t; + async->data = this; #if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10 - uv_async_init(loop, notifyAsync, [](uv_async_t *async, int) { notifyCallback(async); }); + uv_async_init(loop, async, [](uv_async_t *a, int) { reinterpret_cast(a->data)->notifyCallback(); }); #else - uv_async_init(loop, notifyAsync, notifyCallback); + uv_async_init(loop, async, [](uv_async_t *a) { reinterpret_cast(a->data)->notifyCallback(); }); #endif } } -void Request::notifyCallback(uv_async_t *async) { - auto request = reinterpret_cast(async->data); - uv::close(async); - assert(request); - MBGL_VERIFY_THREAD(request->tid) - - if (!request->destructAsync) { - // Call the callback with the result data. This will also delete this object. We haven't - // created a cancel request, so this is safe since it won't be accessed in the future. - // It is up to the user to not call cancel() on this Request object after the response was - // delivered. - request->invoke(); +// Called in the originating thread. +void Request::notifyCallback() { + MBGL_VERIFY_THREAD(tid) + if (!canceled) { + invoke(); } else { - // Otherwise, we're waiting for for the destruct notification to be delivered in order - // to delete the Request object. We're doing this since we can't know whether the - // DefaultFileSource is still sending a cancel event, which means this object must still - // exist. + bool destroy = false; + { + std::unique_lock lock(canceled->mutex); + destroy = canceled->confirmed; + } + // Don't delete right way, because we have to unlock the mutex before deleting. + if (destroy) { + delete this; + } } } @@ -59,15 +60,21 @@ void Request::invoke() { } Request::~Request() { + if (async) { + uv_close(reinterpret_cast(async), [](uv_handle_t* handle) { + delete reinterpret_cast(handle); + }); + } } +// Called in the FileSource thread. void Request::notify(const std::shared_ptr &response_) { + assert(!response); response = response_; assert(response); - if (notifyAsync) { - assert(!notifyAsync->data); - notifyAsync->data = this; - uv_async_send(notifyAsync); + + if (async) { + uv_async_send(async); } else { // This request is not cancelable. This means that the callback will be executed in an // arbitrary thread (== FileSource thread). @@ -75,45 +82,25 @@ void Request::notify(const std::shared_ptr &response_) { } } +// Called in the originating thread. void Request::cancel() { MBGL_VERIFY_THREAD(tid) - assert(notifyAsync); - assert(!destructAsync); - destructAsync = new uv_async_t; - destructAsync->data = nullptr; -#if UV_VERSION_MAJOR == 0 && UV_VERSION_MINOR <= 10 - uv_async_init(notifyAsync->loop, destructAsync, [](uv_async_t *async, int) { cancelCallback(async); }); -#else - uv_async_init(notifyAsync->loop, destructAsync, cancelCallback); -#endif + assert(async); + assert(!canceled); + canceled = util::make_unique(); } -void Request::cancelCallback(uv_async_t *async) { - // The destructAsync will be invoked *after* the notifyAsync callback has already run. - auto request = reinterpret_cast(async->data); - uv::close(async); - assert(request); - MBGL_VERIFY_THREAD(request->tid) - delete request; -} -// This gets called from the FileSource thread, and will only ever be invoked after cancel() was called -// in the original requesting thread. +// Called in the FileSource thread. +// Will only ever be invoked after cancel() was called in the original requesting thread. void Request::destruct() { - assert(notifyAsync); - assert(destructAsync); - - if (!notifyAsync->data) { - // The async hasn't been triggered yet, but we need to so that it'll close the handle. The - // callback will not delete this object since we have a destructAsync handle as well. - notifyAsync->data = this; - uv_async_send(notifyAsync); - } - - // This will finally destruct this object. - assert(!destructAsync->data); - destructAsync->data = this; - uv_async_send(destructAsync); + assert(async); + assert(canceled); + std::unique_lock lock(canceled->mutex); + canceled->confirmed = true; + uv_async_send(async); + // after this method returns, the FileSource thread has no knowledge of + // this object anymore. } }