From 14083700e5e85119adf134e6edbcbfd54cfd33b7 Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Thu, 28 May 2015 10:42:46 +0300 Subject: [PATCH] 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/style/mock_file_source.cpp | 135 ++++++++++++++++++++++++------- test/style/mock_file_source.hpp | 31 +++++-- test/style/pending_resources.cpp | 61 ++++++++++++++ test/test.gypi | 1 + 4 files changed, 191 insertions(+), 37 deletions(-) create mode 100644 test/style/pending_resources.cpp diff --git a/test/style/mock_file_source.cpp b/test/style/mock_file_source.cpp index 42067a2a7..0f2d93f71 100644 --- a/test/style/mock_file_source.cpp +++ b/test/style/mock_file_source.cpp @@ -5,78 +5,151 @@ #include #include +#include + +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 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 pendingRequests_; + uv::timer timer_; + + std::function requestEnqueuedCallback_; }; -void MockFileSource::Impl::replyWithFailure(Response* res) const { - res->status = Response::Status::Error; - res->message = "Failed by the test case"; -} - -void MockFileSource::Impl::replyWithCorruptedData(Response* res, const std::string& url) const { +void MockFileSource::Impl::replyWithSuccess(Request* req) const { + std::shared_ptr res = std::make_shared(); 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::handleRequest(Request* req) const { - const std::string& url = req->resource.url; - std::shared_ptr response = std::make_shared(); - - if (url.find(match_) == std::string::npos) { - replyWithSuccess(response.get(), url); - req->notify(response); +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::replyWithFailure(Request* req) const { + if (req->resource.url.find(match_) == std::string::npos) { + replyWithSuccess(req); + return; + } + + std::shared_ptr res = std::make_shared(); + res->status = Response::Status::Error; + res->message = "Failed by the test case"; + + 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 res = std::make_shared(); + 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>("FileSource", util::ThreadPriority::Low, type, match)) { } +void MockFileSource::setOnRequestDelayedCallback(std::function 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); } } diff --git a/test/style/mock_file_source.hpp b/test/style/mock_file_source.hpp index bb9fb55a3..6bee95dcf 100644 --- a/test/style/mock_file_source.hpp +++ b/test/style/mock_file_source.hpp @@ -2,22 +2,37 @@ #define TEST_RESOURCES_MOCK_FILE_SOURCE #include +#include #include #include namespace mbgl { -namespace util { -template 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 @@ public: 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 callback); + // FileSource implementation. Request* request(const Resource&, uv_loop_t*, Callback) override; void cancel(Request*) override; diff --git a/test/style/pending_resources.cpp b/test/style/pending_resources.cpp new file mode 100644 index 000000000..5d13d49a7 --- /dev/null +++ b/test/style/pending_resources.cpp @@ -0,0 +1,61 @@ +#include "../fixtures/fixture_log_observer.hpp" +#include "../fixtures/util.hpp" +#include "mock_file_source.hpp" + +#include +#include +#include +#include +#include +#include +#include + +using namespace mbgl; + +class PendingResources : public ::testing::TestWithParam { +}; + +// 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(PendingResources, 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(); + HeadlessView view(display); + MockFileSource fileSource(MockFileSource::SuccessWithDelay, GetParam()); + + std::unique_ptr map = std::make_unique(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) { + 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(Style, PendingResources, + ::testing::Values("source.json", "sprite.json", "sprite.png", "sprite", "vector.pbf", "glyphs.pbf", "resources")); diff --git a/test/test.gypi b/test/test.gypi index 0e14ac083..f2407f505 100644 --- a/test/test.gypi +++ b/test/test.gypi @@ -77,6 +77,7 @@ 'style/mock_file_source.cpp', 'style/mock_file_source.hpp', 'style/mock_view.hpp', + 'style/pending_resources.cpp', 'style/resource_loading.cpp', ], 'libraries': [