diff --git a/.clang-format b/.clang-format index 86d562e74..109b562b5 100644 --- a/.clang-format +++ b/.clang-format @@ -2,7 +2,7 @@ Standard: Cpp11 IndentWidth: 4 AccessModifierOffset: -4 UseTab: Never -BinPackParameters: false +BinPackParameters: true AllowShortIfStatementsOnASingleLine: false AllowShortLoopsOnASingleLine: false AllowShortBlocksOnASingleLine: false diff --git a/gyp/http-curl.gypi b/gyp/http-curl.gypi index bfb2054d5..c97ad370b 100644 --- a/gyp/http-curl.gypi +++ b/gyp/http-curl.gypi @@ -14,6 +14,25 @@ '../include', ], + 'variables': { + 'cflags_cc': [ + '<@(uv_cflags)', + '<@(curl_cflags)', + '<@(boost_cflags)', + ], + 'ldflags': [ + '<@(uv_ldflags)', + '<@(curl_ldflags)', + ], + 'libraries': [ + '<@(uv_static_libs)', + '<@(curl_static_libs)', + ], + 'defines': [ + '-DMBGL_HTTP_CURL' + ], + }, + 'conditions': [ ['host == "android"', { 'variables': { @@ -32,19 +51,17 @@ }], ], - 'variables': { - 'cflags_cc': [ - '<@(uv_cflags)', - '<@(curl_cflags)', - '<@(boost_cflags)', - ], - 'ldflags': [ - '<@(uv_ldflags)', - '<@(curl_ldflags)', - ], - 'libraries': [ - '<@(uv_static_libs)', - '<@(curl_static_libs)', + 'direct_dependent_settings': { + 'conditions': [ + ['OS == "mac"', { + 'xcode_settings': { + 'OTHER_CFLAGS': [ '<@(defines)' ], + 'OTHER_CPLUSPLUSFLAGS': [ '<@(defines)' ], + } + }, { + 'cflags': [ '<@(defines)' ], + 'cflags_cc': [ '<@(defines)' ], + }] ], }, diff --git a/gyp/http-nsurl.gypi b/gyp/http-nsurl.gypi index 4205f59d8..5a079fdee 100644 --- a/gyp/http-nsurl.gypi +++ b/gyp/http-nsurl.gypi @@ -25,6 +25,9 @@ 'libraries': [ '<@(uv_static_libs)', ], + 'defines': [ + '-DMBGL_HTTP_NSURL' + ], }, 'xcode_settings': { @@ -32,6 +35,13 @@ 'CLANG_ENABLE_OBJC_ARC': 'NO', }, + 'direct_dependent_settings': { + 'xcode_settings': { + 'OTHER_CFLAGS': [ '<@(defines)' ], + 'OTHER_CPLUSPLUSFLAGS': [ '<@(defines)' ], + } + }, + 'link_settings': { 'libraries': [ '<@(libraries)' ], 'xcode_settings': { diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index 8711aa3d0..00719aa38 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -36,11 +36,7 @@ class View; class GlyphAtlas; class SpriteAtlas; class LineAtlas; - -struct exception : std::runtime_error { - inline exception(const char *msg) : std::runtime_error(msg) { - } -}; +class Environment; class Map : private util::noncopyable { friend class View; @@ -182,8 +178,8 @@ private: Mode mode = Mode::None; -public: // TODO: make private again - std::unique_ptr loop; + const std::unique_ptr env; + View &view; private: std::unique_ptr workers; @@ -214,12 +210,8 @@ private: // Stores whether the map thread has been stopped already. std::atomic_bool isStopped; - View &view; - -#ifdef DEBUG const std::thread::id mainThread; std::thread::id mapThread; -#endif Transform transform; TransformState state; diff --git a/include/mbgl/storage/default/request.hpp b/include/mbgl/storage/default/request.hpp index 648585f30..b686d1fe9 100644 --- a/include/mbgl/storage/default/request.hpp +++ b/include/mbgl/storage/default/request.hpp @@ -15,13 +15,14 @@ typedef struct uv_loop_s uv_loop_t; namespace mbgl { class Response; +class Environment; class Request : private util::noncopyable { MBGL_STORE_THREAD(tid) public: using Callback = std::function; - Request(const Resource &resource, uv_loop_t *loop, Callback callback); + Request(const Resource &resource, uv_loop_t *loop, const Environment &env, Callback callback); public: // May be called from any thread. @@ -45,6 +46,11 @@ private: public: const Resource resource; + + // The environment ref is used to associate requests with a particular environment. This allows + // us to only terminate requests associated with that environment, e.g. when the map the env + // belongs to is discarded. + const Environment &env; }; } diff --git a/include/mbgl/storage/default/shared_request_base.hpp b/include/mbgl/storage/default/shared_request_base.hpp index 2d5661560..59e38efc2 100644 --- a/include/mbgl/storage/default/shared_request_base.hpp +++ b/include/mbgl/storage/default/shared_request_base.hpp @@ -4,11 +4,13 @@ #include #include #include +#include #include #include #include #include +#include #include typedef struct uv_loop_s uv_loop_t; @@ -45,20 +47,12 @@ public: observers.insert(request); } - void unsubscribeAll() { - MBGL_VERIFY_THREAD(tid); - - source = nullptr; - observers.clear(); - cancel(); - } - void unsubscribe(Request *request) { MBGL_VERIFY_THREAD(tid); observers.erase(request); - if (observers.empty()) { + if (abandoned()) { // There are no observers anymore. We are initiating cancelation. if (source) { // First, remove this SharedRequestBase from the source. @@ -70,6 +64,29 @@ public: } } + bool abandoned() const { + return observers.empty(); + } + + std::vector removeAllInEnvironment(const Environment &env) { + MBGL_VERIFY_THREAD(tid); + + std::vector result; + + // Removes all Requests in the supplied environment and returns a list + // of them. + util::erase_if(observers, [&](Request *req) -> bool { + if (&req->env == &env) { + result.push_back(req); + return true; + } else { + return false; + } + }); + + return result; + } + protected: virtual ~SharedRequestBase() { MBGL_VERIFY_THREAD(tid); diff --git a/include/mbgl/storage/default_file_source.hpp b/include/mbgl/storage/default_file_source.hpp index 86e241404..14b4db6ef 100644 --- a/include/mbgl/storage/default_file_source.hpp +++ b/include/mbgl/storage/default_file_source.hpp @@ -20,11 +20,14 @@ class DefaultFileSource : public FileSource { public: DefaultFileSource(FileCache *cache, const std::string &root = ""); DefaultFileSource(FileCache *cache, uv_loop_t *loop, const std::string &root = ""); - ~DefaultFileSource(); + ~DefaultFileSource() override; - Request *request(const Resource &resource, uv_loop_t *loop, Callback callback); - void cancel(Request *request); - void request(const Resource &resource, Callback callback); + Request *request(const Resource &resource, uv_loop_t *loop, const Environment &env, + Callback callback) override; + void cancel(Request *request) override; + void request(const Resource &resource, const Environment &env, Callback callback) override; + + void abort(const Environment &env) override; enum class CacheHint : uint8_t { Full, Refresh, No }; void notify(SharedRequestBase *sharedRequest, const std::set &observers, @@ -39,14 +42,16 @@ private: struct RemoveRequestAction; struct ResultAction; struct StopAction; - using Action = - mapbox::util::variant; + struct AbortAction; + using Action = mapbox::util::variant; using Queue = util::AsyncQueue; void process(AddRequestAction &action); void process(RemoveRequestAction &action); void process(ResultAction &action); void process(StopAction &action); + void process(AbortAction &action); SharedRequestBase *find(const Resource &resource); diff --git a/include/mbgl/storage/file_source.hpp b/include/mbgl/storage/file_source.hpp index 8517d6e4a..30e88c39f 100644 --- a/include/mbgl/storage/file_source.hpp +++ b/include/mbgl/storage/file_source.hpp @@ -15,6 +15,7 @@ typedef struct uv_loop_s uv_loop_t; namespace mbgl { class Request; +class Environment; class FileSource : private util::noncopyable { protected: @@ -27,12 +28,19 @@ public: // These can be called from any thread. The callback will be invoked in the loop. // You can only cancel a request from the same thread it was created in. - virtual Request *request(const Resource &resource, uv_loop_t *loop, Callback callback) = 0; + virtual Request *request(const Resource &resource, uv_loop_t *loop, const Environment &env, + Callback callback) = 0; virtual void cancel(Request *request) = 0; // These can be called from any thread. The callback will be invoked in an arbitrary other thread. // You cannot cancel these requests. - virtual void request(const Resource &resource, Callback callback) = 0; + virtual void request(const Resource &resource, const Environment &env, Callback callback) = 0; + + // This can be called from any thread. All requests with the environment pointer env should be + // notified as errored. Note that this is /different/ from canceling requests; a canceled + // request's callback is never called, while an aborted request's callback is called with + // a error message. + virtual void abort(const Environment &env) = 0; }; } diff --git a/platform/darwin/http_request_nsurl.mm b/platform/darwin/http_request_nsurl.mm index 83c010f8b..638b5062d 100644 --- a/platform/darwin/http_request_nsurl.mm +++ b/platform/darwin/http_request_nsurl.mm @@ -188,6 +188,7 @@ void HTTPRequestImpl::handleResponse() { } context->removeRequest(request); + request->ptr = nullptr; delete request; request = nullptr; } @@ -206,6 +207,8 @@ void HTTPRequestImpl::cancel() { [task cancel]; [task release]; task = nullptr; + } else { + delete this; } } diff --git a/src/mbgl/map/environment.cpp b/src/mbgl/map/environment.cpp new file mode 100644 index 000000000..ee13d33ea --- /dev/null +++ b/src/mbgl/map/environment.cpp @@ -0,0 +1,39 @@ +#include +#include + +#include + +#include + +namespace mbgl { + +Environment::Environment(FileSource &fs) : fileSource(fs), loop(uv_loop_new()) { +} + +void Environment::setup() { + mapThread = std::this_thread::get_id(); +} + +bool Environment::inMapThread() const { + return std::this_thread::get_id() == mapThread; +} + +void Environment::requestAsync(const Resource &resource, std::function callback) { + fileSource.request(resource, *this, std::move(callback)); +} + +Request *Environment::request(const Resource &resource, std::function callback) { + assert(inMapThread()); + return fileSource.request(resource, loop, *this, std::move(callback)); +} + +void Environment::cancelRequest(Request *req) { + assert(inMapThread()); + fileSource.cancel(req); +} + +void Environment::terminate() { + fileSource.abort(*this); +} + +} diff --git a/src/mbgl/map/environment.hpp b/src/mbgl/map/environment.hpp new file mode 100644 index 000000000..b68a7b9e2 --- /dev/null +++ b/src/mbgl/map/environment.hpp @@ -0,0 +1,44 @@ +#ifndef MBGL_MAP_MAP_ENVIRONMENT +#define MBGL_MAP_MAP_ENVIRONMENT + +#include +#include + +#include +#include + +typedef struct uv_loop_s uv_loop_t; + +namespace mbgl { + +class FileSource; +class Request; +class Response; +struct Resource; + +class Environment : private util::noncopyable { +public: + Environment(FileSource &); + + void setup(); + + bool inMapThread() const; + + void requestAsync(const Resource &, std::function); + Request *request(const Resource &, std::function); + void cancelRequest(Request *); + + // Request to terminate the environment. + void terminate(); + +private: + FileSource &fileSource; + std::thread::id mapThread; + +public: + uv_loop_t *const loop; +}; + +} + +#endif diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 518bc5bb9..0cd7d3621 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include #include +#include #include #include @@ -57,16 +59,14 @@ const static bool uvVersionCheck = []() { using namespace mbgl; Map::Map(View& view_, FileSource& fileSource_) - : loop(util::make_unique()), + : env(util::make_unique(fileSource_)), view(view_), -#ifdef DEBUG mainThread(std::this_thread::get_id()), mapThread(mainThread), -#endif transform(view_), fileSource(fileSource_), glyphAtlas(util::make_unique(1024, 1024)), - glyphStore(std::make_shared(fileSource)), + glyphStore(std::make_shared(*env)), spriteAtlas(util::make_unique(512, 512)), lineAtlas(util::make_unique(512, 512)), texturePool(std::make_shared()), @@ -92,7 +92,7 @@ Map::~Map() { texturePool.reset(); workers.reset(); - uv_run(**loop, UV_RUN_DEFAULT); + uv_run(env->loop, UV_RUN_DEFAULT); } uv::worker &Map::getWorker() { @@ -112,7 +112,7 @@ void Map::start(bool startPaused) { isStopped = false; // Setup async notifications - asyncTerminate = util::make_unique(**loop, [this]() { + asyncTerminate = util::make_unique(env->loop, [this]() { assert(std::this_thread::get_id() == mapThread); // Remove all of these to make sure they are destructed in the correct thread. @@ -127,7 +127,7 @@ void Map::start(bool startPaused) { asyncTerminate.reset(); }); - asyncRender = util::make_unique(**loop, [this]() { + asyncRender = util::make_unique(env->loop, [this]() { assert(std::this_thread::get_id() == mapThread); if (state.hasSize()) { @@ -207,7 +207,7 @@ void Map::pause(bool waitForPause) { pausing = true; mutexRun.unlock(); - uv_stop(**loop); + uv_stop(env->loop); rerender(); // Needed to ensure uv_stop is seen and uv_run exits, otherwise we deadlock on wait_for_pause if (waitForPause) { @@ -242,12 +242,14 @@ void Map::run() { } if (mode == Mode::Static && !style && styleURL.empty()) { - throw exception("Style is not set"); + throw util::Exception("Style is not set"); } view.activate(); - workers = util::make_unique(**loop, 4, "Tile Worker"); + workers = util::make_unique(env->loop, 4, "Tile Worker"); + + env->setup(); setup(); prepare(); @@ -255,15 +257,15 @@ void Map::run() { if (mode == Mode::Continuous) { terminating = false; while(!terminating) { - uv_run(**loop, UV_RUN_DEFAULT); + uv_run(env->loop, UV_RUN_DEFAULT); checkForPause(); } } else { - uv_run(**loop, UV_RUN_DEFAULT); + uv_run(env->loop, UV_RUN_DEFAULT); } // Run the event loop once more to make sure our async delete handlers are called. - uv_run(**loop, UV_RUN_ONCE); + uv_run(env->loop, UV_RUN_ONCE); // If the map rendering wasn't started asynchronously, we perform one render // *after* all events have been processed. @@ -376,7 +378,7 @@ util::ptr Map::getSprite() { const float pixelRatio = state.getPixelRatio(); const std::string &sprite_url = style->getSpriteURL(); if (!sprite || sprite->pixelRatio != pixelRatio) { - sprite = Sprite::Create(sprite_url, pixelRatio, fileSource); + sprite = Sprite::Create(sprite_url, pixelRatio, *env); } return sprite; @@ -621,7 +623,7 @@ void Map::updateSources() { if (source->enabled) { if (!source->source) { source->source = std::make_shared(source->info); - source->source->load(*this, fileSource); + source->source->load(*this, *env); } } else { source->source.reset(); @@ -648,10 +650,8 @@ void Map::updateSources(const util::ptr &group) { void Map::updateTiles() { for (const auto& source : activeSources) { - source->source->update(*this, getWorker(), - style, *glyphAtlas, *glyphStore, - *spriteAtlas, getSprite(), - *texturePool, fileSource, ***loop, [this](){ update(); }); + source->source->update(*this, *env, getWorker(), style, *glyphAtlas, *glyphStore, + *spriteAtlas, getSprite(), *texturePool, [this]() { update(); }); } } @@ -659,7 +659,7 @@ void Map::prepare() { if (!style) { style = std::make_shared