Cancel all pending requests before leaving

This still has the unwanted side effect off blocking the Map thread
waiting for the workers to return (because they hold a reference
to the Map thread main loop).

Maybe the only way to solve this would be having a I/O thread for
dispatching messages.

The Request is not bound anymore to the invoker thread because the workers
will request for glyphs, but the reply will arrive at the Map thread.

7 files changed, 37 insertions, 16 deletions

diff --git a/src/mbgl/map/environment.cpp b/src/mbgl/map/environment.cpp
index dd6c1f1933..5857d4bdf6 100644
--- a/src/mbgl/map/environment.cpp
+++ b/src/mbgl/map/environment.cpp
@@ -128,7 +128,6 @@ void Environment::requestAsync(const Resource& resource,
 
 Request* Environment::request(const Resource& resource,
                               std::function<void(const Response&)> callback) {
-    assert(currentlyOn(ThreadType::Map));
     return fileSource.request(resource, util::RunLoop::current.get()->get(), std::move(callback));
 }
 
diff --git a/src/mbgl/map/resource_loader.cpp b/src/mbgl/map/resource_loader.cpp
index 5dc0ab9d62..b3aa35b74a 100644
--- a/src/mbgl/map/resource_loader.cpp
+++ b/src/mbgl/map/resource_loader.cpp
@@ -37,10 +37,9 @@ void ResourceLoader::setStyle(Style* style) {
 
     style_ = style;
 
-    Environment& env = Environment::Get();
     for (const auto& source : style->sources) {
         source->setObserver(this);
-        source->load(accessToken_, env);
+        source->load(accessToken_);
     }
 }
 
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index 9b6631da9e..8b0bf905ff 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -113,11 +113,13 @@ std::string SourceInfo::tileURL(const TileID& id, float pixelRatio) const {
     return result;
 }
 
-Source::Source()
-{
-}
+Source::Source() {}
 
-Source::~Source() {}
+Source::~Source() {
+    if (req) {
+        Environment::Get().cancelRequest(req);
+    }
+}
 
 bool Source::isLoaded() const {
     if (!loaded) {
@@ -136,7 +138,7 @@ bool Source::isLoaded() const {
 // Note: This is a separate function that must be called exactly once after creation
 // The reason this isn't part of the constructor is that calling shared_from_this() in
 // the constructor fails.
-void Source::load(const std::string& accessToken, Environment& env) {
+void Source::load(const std::string& accessToken) {
     if (info.url.empty()) {
         loaded = true;
         return;
@@ -145,7 +147,9 @@ void Source::load(const std::string& accessToken, Environment& env) {
     util::ptr<Source> source = shared_from_this();
 
     const std::string url = util::mapbox::normalizeSourceURL(info.url, accessToken);
-    env.request({ Resource::Kind::JSON, url }, [source](const Response &res) {
+    req = Environment::Get().request({ Resource::Kind::JSON, url }, [source](const Response &res) {
+        source->clearRequest();
+
         if (res.status != Response::Successful) {
             Log::Warning(Event::General, "Failed to load source TileJSON: %s", res.message.c_str());
             return;
@@ -505,6 +509,10 @@ void Source::onLowMemory() {
     cache.clear();
 }
 
+void Source::clearRequest() {
+    req = nullptr;
+}
+
 void Source::setObserver(Observer* observer) {
     observer_ = observer;
 }
diff --git a/src/mbgl/map/source.hpp b/src/mbgl/map/source.hpp
index dd02cdc3ba..eb7950225d 100644
--- a/src/mbgl/map/source.hpp
+++ b/src/mbgl/map/source.hpp
@@ -29,6 +29,7 @@ class Sprite;
 class TexturePool;
 class Style;
 class Painter;
+class Request;
 class TransformState;
 class Tile;
 struct ClipID;
@@ -63,7 +64,7 @@ public:
     Source();
     ~Source();
 
-    void load(const std::string& accessToken, Environment&);
+    void load(const std::string& accessToken);
     bool isLoaded() const;
 
     void load(MapData&, Environment&, std::function<void()> callback);
@@ -87,6 +88,7 @@ public:
 
     void setCacheSize(size_t);
     void onLowMemory();
+    void clearRequest();
 
     void setObserver(Observer* observer);
 
@@ -128,7 +130,7 @@ private:
     std::map<TileID, std::weak_ptr<TileData>> tile_data;
     TileCache cache;
 
-
+    Request* req = nullptr;
     Observer* observer_ = nullptr;
 };
 
diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp
index d413cabbe7..53a882f2a6 100644
--- a/src/mbgl/storage/request.cpp
+++ b/src/mbgl/storage/request.cpp
@@ -1,5 +1,6 @@
 #include <mbgl/storage/request.hpp>
 
+#include <mbgl/map/environment.hpp>
 #include <mbgl/storage/response.hpp>
 
 #include <mbgl/util/util.hpp>
@@ -33,7 +34,6 @@ Request::Request(const Resource &resource_, uv_loop_t *loop, Callback callback_)
 
 // Called in the originating thread.
 void Request::notifyCallback() {
-    MBGL_VERIFY_THREAD(tid)
     if (!canceled) {
         invoke();
     } else {
@@ -84,7 +84,6 @@ void Request::notify(const std::shared_ptr<const Response> &response_) {
 
 // Called in the originating thread.
 void Request::cancel() {
-    MBGL_VERIFY_THREAD(tid)
     assert(async);
     assert(!canceled);
     canceled = util::make_unique<Canceled>();
diff --git a/src/mbgl/text/glyph_store.cpp b/src/mbgl/text/glyph_store.cpp
index 80af0db98f..71c1be6d95 100644
--- a/src/mbgl/text/glyph_store.cpp
+++ b/src/mbgl/text/glyph_store.cpp
@@ -142,9 +142,9 @@ void FontStack::lineWrap(Shaping &shaping, const float lineHeight, const float m
 GlyphPBF::GlyphPBF(const std::string& glyphURL,
                    const std::string& fontStack,
                    GlyphRange glyphRange,
-                   Environment& env,
+                   Environment& env_,
                    const GlyphLoadedCallback& callback)
-    : parsed(false) {
+    : parsed(false), env(env_) {
     // Load the glyph set URL
     std::string url = util::replaceTokens(glyphURL, [&](const std::string &name) -> std::string {
         if (name == "fontstack") return util::percentEncode(fontStack);
@@ -153,7 +153,9 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL,
     });
 
     // The prepare call jumps back to the main thread.
-    env.requestAsync({ Resource::Kind::Glyphs, url }, [&, url, callback](const Response &res) {
+    req = env.request({ Resource::Kind::Glyphs, url }, [&, url, callback](const Response &res) {
+        req = nullptr;
+
         if (res.status != Response::Successful) {
             // Something went wrong with loading the glyph pbf.
             const std::string msg = std::string { "[ERROR] failed to load glyphs: " } + url + " message: " + res.message;
@@ -170,6 +172,12 @@ GlyphPBF::GlyphPBF(const std::string& glyphURL,
     });
 }
 
+GlyphPBF::~GlyphPBF() {
+    if (req) {
+        env.cancelRequest(req);
+    }
+}
+
 void GlyphPBF::parse(FontStack &stack) {
     std::lock_guard<std::mutex> lock(mtx);
 
diff --git a/src/mbgl/text/glyph_store.hpp b/src/mbgl/text/glyph_store.hpp
index 6b7e2b8bd5..a41eec5330 100644
--- a/src/mbgl/text/glyph_store.hpp
+++ b/src/mbgl/text/glyph_store.hpp
@@ -17,6 +17,7 @@ namespace mbgl {
 
 class FileSource;
 class Environment;
+class Request;
 
 class SDFGlyph {
 public:
@@ -56,6 +57,7 @@ public:
              GlyphRange glyphRange,
              Environment &env,
              const GlyphLoadedCallback& callback);
+    ~GlyphPBF();
 
     void parse(FontStack &stack);
     bool isParsed() const;
@@ -68,6 +70,10 @@ private:
 
     std::string data;
     std::atomic_bool parsed;
+
+    Environment& env;
+    Request* req = nullptr;
+
     mutable std::mutex mtx;
 };