Test Map object destruction with pending requests

Test whenever destroying a Map object with requests pending
works and is done quickly.

The test injects artificial delays in selected requests and
tries to destroy the Map object afterwards.

Currently glyph requests are skipped because there is a bug
being worked on in a different issue.

test/resources/mock_file_source.cpp

@@ -5,78 +5,151 @@
 #include <mbgl/util/io.hpp>
 #include <mbgl/util/thread.hpp>
 
+#include <algorithm>
+
+namespace {
+
+const uint64_t timeout = 1000000;
+
+}
+
 namespace mbgl {
 
 class MockFileSource::Impl {
 public:
-    Impl(uv_loop_t*, Type type, const std::string& match) : type_(type), match_(match) {}
+    Impl(uv_loop_t* loop, Type type, const std::string& match)
+        : type_(type), match_(match), timer_(loop) {
+        timer_.start(timeout, timeout, [this] { dispatchPendingRequests(); });
+        timer_.unref();
+    }
 
-    void handleRequest(Request* req) const;
+    ~Impl() {
+        timer_.stop();
+    }
+
+    void setOnRequestDelayedCallback(std::function<void(void)> callback) {
+        requestEnqueuedCallback_ = callback;
+    }
+
+    void handleRequest(Request* req);
+    void cancelRequest(Request* req);
 
 private:
-    void replyWithFailure(Response* res) const;
-    void replyWithCorruptedData(Response* res, const std::string& url) const;
-    void replyWithSuccess(Response* res, const std::string& url) const;
+    void replyWithSuccess(Request* req) const;
+    void replyWithSuccessWithDelay(Request* req);
+    void replyWithFailure(Request* req) const;
+    void replyWithCorruptedData(Request* req) const;
+
+    void dispatchPendingRequests();
 
     Type type_;
     std::string match_;
-};
+    std::vector<Request*> pendingRequests_;
+    uv::timer timer_;
 
-void MockFileSource::Impl::replyWithFailure(Response* res) const {
-    res->status = Response::Status::Error;
-    res->message = "Failed by the test case";
-}
+    std::function<void(void)> requestEnqueuedCallback_;
+};
 
-void MockFileSource::Impl::replyWithCorruptedData(Response* res, const std::string& url) const {
+void MockFileSource::Impl::replyWithSuccess(Request* req) const {
+    std::shared_ptr<Response> res = std::make_shared<Response>();
     res->status = Response::Status::Successful;
-    res->data = util::read_file(url);
-    res->data.insert(0, "CORRUPTED");
+    res->data = util::read_file(req->resource.url);
+
+    req->notify(res);
 }
 
-void MockFileSource::Impl::replyWithSuccess(Response* res, const std::string& url) const {
-    res->status = Response::Status::Successful;
-    res->data = util::read_file(url);
+void MockFileSource::Impl::replyWithSuccessWithDelay(Request* req) {
+    if (req->resource.url.find(match_) == std::string::npos) {
+        replyWithSuccess(req);
+        return;
+    }
+
+    pendingRequests_.push_back(req);
+    requestEnqueuedCallback_();
 }
 
-void MockFileSource::Impl::handleRequest(Request* req) const {
-    const std::string& url = req->resource.url;
-    std::shared_ptr<Response> response = std::make_shared<Response>();
+void MockFileSource::Impl::replyWithFailure(Request* req) const {
+    if (req->resource.url.find(match_) == std::string::npos) {
+        replyWithSuccess(req);
+        return;
+    }
+
+    std::shared_ptr<Response> res = std::make_shared<Response>();
+    res->status = Response::Status::Error;
+    res->message = "Failed by the test case";
 
-    if (url.find(match_) == std::string::npos) {
-        replyWithSuccess(response.get(), url);
-        req->notify(response);
+    req->notify(res);
+}
+
+void MockFileSource::Impl::replyWithCorruptedData(Request* req) const {
+    if (req->resource.url.find(match_) == std::string::npos) {
+        replyWithSuccess(req);
         return;
     }
 
+    std::shared_ptr<Response> res = std::make_shared<Response>();
+    res->status = Response::Status::Successful;
+    res->data = util::read_file(req->resource.url);
+    res->data.insert(0, "CORRUPTED");
+
+    req->notify(res);
+}
+
+void MockFileSource::Impl::handleRequest(Request* req) {
     switch (type_) {
     case Type::Success:
-        replyWithSuccess(response.get(), url);
+        replyWithSuccess(req);
+        break;
+    case Type::SuccessWithDelay:
+        replyWithSuccessWithDelay(req);
         break;
     case Type::RequestFail:
-        replyWithFailure(response.get());
+        replyWithFailure(req);
         break;
     case Type::RequestWithCorruptedData:
-        replyWithCorruptedData(response.get(), url);
+        replyWithCorruptedData(req);
         break;
     default:
         EXPECT_TRUE(false) << "Should never be reached.";
     }
+}
 
-    req->notify(response);
+void MockFileSource::Impl::cancelRequest(Request* req) {
+    auto it = std::find(pendingRequests_.begin(), pendingRequests_.end(), req);
+    if (it != pendingRequests_.end()) {
+        (*it)->destruct();
+        pendingRequests_.erase(it);
+    } else {
+        EXPECT_TRUE(false) << "Should never be reached.";
+    }
+}
+
+void MockFileSource::Impl::dispatchPendingRequests() {
+    for (auto req : pendingRequests_) {
+        replyWithSuccess(req);
+    }
+
+    pendingRequests_.clear();
 }
 
 MockFileSource::MockFileSource(Type type, const std::string& match)
     : thread_(std::make_unique<util::Thread<Impl>>("FileSource", util::ThreadPriority::Low, type, match)) {
 }
 
+void MockFileSource::setOnRequestDelayedCallback(std::function<void(void)> callback) {
+    thread_->invokeSync(&Impl::setOnRequestDelayedCallback, callback);
+}
+
 Request* MockFileSource::request(const Resource& resource, uv_loop_t* loop, Callback callback) {
     Request* req = new Request(resource, loop, std::move(callback));
-    thread_->invoke(&Impl::handleRequest, req);
+    thread_->invokeSync(&Impl::handleRequest, req);
 
     return req;
 }
 
-void MockFileSource::cancel(Request*) {
+void MockFileSource::cancel(Request* req) {
+    req->cancel();
+    thread_->invoke(&Impl::cancelRequest, req);
 }
 
 }

test/resources/mock_file_source.hpp

@@ -2,22 +2,37 @@
 #define TEST_RESOURCES_MOCK_FILE_SOURCE
 
 #include <mbgl/storage/file_source.hpp>
+#include <mbgl/util/thread.hpp>
 
 #include <string>
 #include <memory>
 
 namespace mbgl {
 
-namespace util {
-template <typename T> class Thread;
-}
-
-// This mock FileSource will read data from the disk and will fail
-// the request if the URL matches a string.
+// The MockFileSource is a FileSource that can simulate different
+// types of failures and it will work completely offline.
 class MockFileSource : public FileSource {
 public:
+    // Success:
+    //     Will reply to every request correctly with valid data.
+    //
+    // SuccessWithDelay:
+    //     Will reply to every request correctly with valid data,
+    //     but the ones that contains the "match" string on the
+    //     URL will be answered after a delay. This can be useful
+    //     for testing request cancellation.
+    //
+    // RequestFail:
+    //     Will reply with an error to requests that contains
+    //     the "match" string on the URL.
+    //
+    // RequestWithCorruptedData:
+    //     Will answer every request successfully but will return
+    //     corrupt data on the requests that contains the "match"
+    //     string on the URL.
     enum Type {
         Success,
+        SuccessWithDelay,
         RequestFail,
         RequestWithCorruptedData
     };
@@ -27,6 +42,10 @@ class MockFileSource : public FileSource {
     MockFileSource(Type type, const std::string& match);
     ~MockFileSource() override = default;
 
+    // Function that gets called when a delayed resource is enqueued. The
+    // callback must be safe to call from any thread.
+    void setOnRequestDelayedCallback(std::function<void(void)> callback);
+
     // FileSource implementation.
     Request* request(const Resource&, uv_loop_t*, Callback) override;
     void cancel(Request*) override;

test/resources/pending_resources.cpp

@@ -0,0 +1,61 @@
+#include "../fixtures/fixture_log_observer.hpp"
+#include "../fixtures/util.hpp"
+#include "mock_file_source.hpp"
+
+#include <mbgl/map/map.hpp>
+#include <mbgl/map/still_image.hpp>
+#include <mbgl/platform/default/headless_display.hpp>
+#include <mbgl/platform/default/headless_view.hpp>
+#include <mbgl/util/io.hpp>
+#include <mbgl/util/run_loop.hpp>
+#include <mbgl/util/uv_detail.hpp>
+
+using namespace mbgl;
+
+class PendingResourcesTest : public ::testing::TestWithParam<std::string> {
+};
+
+// This test will load a Style but one of the resources requested will not be
+// replied immediately like the others. We get an notification by the
+// MockFileSource when some resource is artificially delayed and we destroy
+// the Map object after that. The idea here is to test if these pending requests
+// are getting canceled correctly if on shutdown.
+TEST_P(PendingResourcesTest, DeleteMapObjectWithPendingRequest) {
+    // TODO: The glyphs test is blocked by the issue #1664.
+    if (GetParam() == "glyphs.pbf") {
+        return;
+    }
+
+    util::RunLoop loop(uv_default_loop());
+
+    auto display = std::make_shared<mbgl::HeadlessDisplay>();
+    HeadlessView view(display);
+    MockFileSource fileSource(MockFileSource::SuccessWithDelay, GetParam());
+
+    std::unique_ptr<Map> map = std::make_unique<Map>(view, fileSource, MapMode::Still);
+
+    uv::async endTest(loop.get(), [&map, &loop] {
+        map.reset();
+        loop.stop();
+    });
+
+    endTest.unref();
+    fileSource.setOnRequestDelayedCallback([&endTest] { endTest.send(); });
+
+    const std::string style = util::read_file("test/fixtures/resources/style.json");
+    map->resize(1000, 1000, 1.0);
+    map->setStyleJSON(style, ".");
+
+    map->renderStill([&endTest](std::exception_ptr, std::unique_ptr<const StillImage>) {
+        EXPECT_TRUE(false) << "Should never happen.";
+    });
+
+    uv_run(loop.get(), UV_RUN_DEFAULT);
+}
+
+// In the test data below, "sprite" will match both "sprite.json" and "sprite.png" and cause two
+// requests to be canceled. "resources" will match everything but in practice will only test the
+// cancellation of the sprites and "source.json" because we only load the rest after "source.json"
+// gets parsed.
+INSTANTIATE_TEST_CASE_P(ResourceLoader, PendingResourcesTest,
+    ::testing::Values("source.json", "sprite.json", "sprite.png", "sprite", "vector.pbf", "glyphs.pbf", "resources"));

test/test.gypi

@@ -62,6 +62,7 @@
         'resources/mock_file_source.cpp',
         'resources/mock_file_source.hpp',
         'resources/mock_view.hpp',
+        'resources/pending_resources.cpp',
         'resources/resource_loader.cpp',
 
         'storage/storage.hpp',