From 94a5ac857f7cfc0a8e20035fabc39841fe17249c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Thu, 15 Oct 2015 15:10:41 +0200 Subject: [PATCH] [core] all requests have to be canceled explicitly now By not automatically destroying Request objects after the result has been delivered, we are making sure that we can potentially fire the callback multiple times without adverse effects. This means that you have to hold on to the result of fs->request(), can explicitly cancel it if you don't want to be notified of data changes anymore. Not doing so will monitor the request indefinitely and will prevent the app from exiting. --- platform/node/src/node_file_source.cpp | 1 - src/mbgl/map/map_context.cpp | 5 +++-- src/mbgl/map/raster_tile_data.cpp | 1 + src/mbgl/map/source.cpp | 2 ++ src/mbgl/map/sprite.cpp | 4 ++++ src/mbgl/map/vector_tile_data.cpp | 1 + src/mbgl/storage/default_file_source.cpp | 6 ++++++ src/mbgl/storage/request.cpp | 10 +++++----- src/mbgl/text/glyph_pbf.cpp | 2 ++ test/storage/cache_response.cpp | 6 ++++-- test/storage/cache_revalidate.cpp | 18 ++++++++++++------ test/storage/database.cpp | 8 ++++---- test/storage/directory_reading.cpp | 3 ++- test/storage/file_reading.cpp | 9 ++++++--- test/storage/http_cancel.cpp | 3 ++- test/storage/http_coalescing.cpp | 6 +++++- test/storage/http_error.cpp | 6 ++++-- test/storage/http_header_parsing.cpp | 6 ++++-- test/storage/http_issue_1369.cpp | 3 ++- test/storage/http_load.cpp | 14 +++++++++----- test/storage/http_other_loop.cpp | 3 ++- test/storage/http_reading.cpp | 20 ++++++++++++++------ 22 files changed, 94 insertions(+), 43 deletions(-) diff --git a/platform/node/src/node_file_source.cpp b/platform/node/src/node_file_source.cpp index b4828e768..5a5a34f0c 100644 --- a/platform/node/src/node_file_source.cpp +++ b/platform/node/src/node_file_source.cpp @@ -133,7 +133,6 @@ void NodeFileSource::notify(const mbgl::Resource& resource, const std::shared_pt } observersIt->second->notify(response); - observers.erase(observersIt); } } diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp index d3324cb27..573882da8 100644 --- a/src/mbgl/map/map_context.cpp +++ b/src/mbgl/map/map_context.cpp @@ -53,8 +53,7 @@ void MapContext::cleanup() { view.notify(); if (styleRequest) { - FileSource* fs = util::ThreadContext::getFileSource(); - fs->cancel(styleRequest); + util::ThreadContext::getFileSource()->cancel(styleRequest); styleRequest = nullptr; } @@ -100,6 +99,7 @@ void MapContext::setStyleURL(const std::string& url) { if (styleRequest) { fs->cancel(styleRequest); + styleRequest = nullptr; } styleURL = url; @@ -114,6 +114,7 @@ void MapContext::setStyleURL(const std::string& url) { } styleRequest = fs->request({ Resource::Kind::Style, styleURL }, util::RunLoop::getLoop(), [this, base](const Response &res) { + util::ThreadContext::getFileSource()->cancel(styleRequest); styleRequest = nullptr; if (res.status == Response::Successful) { diff --git a/src/mbgl/map/raster_tile_data.cpp b/src/mbgl/map/raster_tile_data.cpp index 27094510a..c983a5737 100644 --- a/src/mbgl/map/raster_tile_data.cpp +++ b/src/mbgl/map/raster_tile_data.cpp @@ -31,6 +31,7 @@ void RasterTileData::request(float pixelRatio, FileSource* fs = util::ThreadContext::getFileSource(); req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::getLoop(), [url, callback, this](const Response &res) { + util::ThreadContext::getFileSource()->cancel(req); req = nullptr; if (res.status == Response::NotFound) { diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp index 49e1d8161..3e8aba885 100644 --- a/src/mbgl/map/source.cpp +++ b/src/mbgl/map/source.cpp @@ -125,6 +125,7 @@ Source::Source() {} Source::~Source() { if (req) { util::ThreadContext::getFileSource()->cancel(req); + req = nullptr; } } @@ -153,6 +154,7 @@ void Source::load() { FileSource* fs = util::ThreadContext::getFileSource(); req = fs->request({ Resource::Kind::Source, info.url }, util::RunLoop::getLoop(), [this](const Response &res) { + util::ThreadContext::getFileSource()->cancel(req); req = nullptr; if (res.status != Response::Successful) { diff --git a/src/mbgl/map/sprite.cpp b/src/mbgl/map/sprite.cpp index d5628b05b..02bf0895c 100644 --- a/src/mbgl/map/sprite.cpp +++ b/src/mbgl/map/sprite.cpp @@ -29,9 +29,11 @@ struct Sprite::Loader { ~Loader() { if (jsonRequest) { util::ThreadContext::getFileSource()->cancel(jsonRequest); + jsonRequest = nullptr; } if (spriteRequest) { util::ThreadContext::getFileSource()->cancel(spriteRequest); + spriteRequest = nullptr; } } }; @@ -52,6 +54,7 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) FileSource* fs = util::ThreadContext::getFileSource(); loader->jsonRequest = fs->request({ Resource::Kind::SpriteJSON, jsonURL }, util::RunLoop::getLoop(), [this, jsonURL](const Response& res) { + util::ThreadContext::getFileSource()->cancel(loader->jsonRequest); loader->jsonRequest = nullptr; if (res.status == Response::Successful) { loader->data->json = res.data; @@ -68,6 +71,7 @@ Sprite::Sprite(const std::string& baseUrl, float pixelRatio_) loader->spriteRequest = fs->request({ Resource::Kind::SpriteImage, spriteURL }, util::RunLoop::getLoop(), [this, spriteURL](const Response& res) { + util::ThreadContext::getFileSource()->cancel(loader->spriteRequest); loader->spriteRequest = nullptr; if (res.status == Response::Successful) { loader->data->image = res.data; diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp index 3fe96599d..a10dd64bd 100644 --- a/src/mbgl/map/vector_tile_data.cpp +++ b/src/mbgl/map/vector_tile_data.cpp @@ -43,6 +43,7 @@ void VectorTileData::request(float pixelRatio, const std::function& call FileSource* fs = util::ThreadContext::getFileSource(); req = fs->request({ Resource::Kind::Tile, url }, util::RunLoop::getLoop(), [url, callback, this](const Response &res) { + util::ThreadContext::getFileSource()->cancel(req); req = nullptr; if (res.status == Response::NotFound) { diff --git a/src/mbgl/storage/default_file_source.cpp b/src/mbgl/storage/default_file_source.cpp index c33728db1..7b3cff825 100644 --- a/src/mbgl/storage/default_file_source.cpp +++ b/src/mbgl/storage/default_file_source.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wshadow" @@ -40,6 +41,10 @@ Request* DefaultFileSource::request(const Resource& resource, Callback callback) { assert(l); + if (!callback) { + throw util::MisuseException("FileSource callback can't be empty"); + } + std::string url; switch (resource.kind) { @@ -70,6 +75,7 @@ Request* DefaultFileSource::request(const Resource& resource, } void DefaultFileSource::cancel(Request *req) { + assert(req); req->cancel(); thread->invoke(&Impl::cancel, req); } diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp index 79d441442..f8ad55537 100644 --- a/src/mbgl/storage/request.cpp +++ b/src/mbgl/storage/request.cpp @@ -6,6 +6,7 @@ #include #include +#include namespace mbgl { @@ -40,18 +41,17 @@ void Request::invoke() { // The user could supply a null pointer or empty std::function as a callback. In this case, we // still do the file request, but we don't need to deliver a result. if (callback) { - callback(*response); + callback(*std::atomic_load(&response)); } - delete this; } Request::~Request() = default; // Called in the FileSource thread. void Request::notify(const std::shared_ptr &response_) { - assert(!response); - response = response_; - assert(response); + assert(!std::atomic_load(&response)); + assert(response_); + std::atomic_store(&response, response_); async->send(); } diff --git a/src/mbgl/text/glyph_pbf.cpp b/src/mbgl/text/glyph_pbf.cpp index ba92ce6b4..f9f8afcb1 100644 --- a/src/mbgl/text/glyph_pbf.cpp +++ b/src/mbgl/text/glyph_pbf.cpp @@ -75,6 +75,7 @@ GlyphPBF::GlyphPBF(GlyphStore* store, }); auto requestCallback = [this, store, fontStack, url](const Response &res) { + util::ThreadContext::getFileSource()->cancel(req); req = nullptr; if (res.status != Response::Successful) { @@ -94,6 +95,7 @@ GlyphPBF::GlyphPBF(GlyphStore* store, GlyphPBF::~GlyphPBF() { if (req) { util::ThreadContext::getFileSource()->cancel(req); + req = nullptr; } } diff --git a/test/storage/cache_response.cpp b/test/storage/cache_response.cpp index 87de62025..0fba2ba5e 100644 --- a/test/storage/cache_response.cpp +++ b/test/storage/cache_response.cpp @@ -15,7 +15,8 @@ TEST_F(Storage, CacheResponse) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/cache" }; - fs.request(resource, uv_default_loop(), [&](const Response &res) { + Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Response 1", res.data); EXPECT_LT(0, res.expires); @@ -23,7 +24,8 @@ TEST_F(Storage, CacheResponse) { EXPECT_EQ("", res.etag); EXPECT_EQ("", res.message); - fs.request(resource, uv_default_loop(), [&, res](const Response &res2) { + req = fs.request(resource, uv_default_loop(), [&, res](const Response &res2) { + fs.cancel(req); EXPECT_EQ(res.status, res2.status); EXPECT_EQ(res.data, res2.data); EXPECT_EQ(res.expires, res2.expires); diff --git a/test/storage/cache_revalidate.cpp b/test/storage/cache_revalidate.cpp index 1bfd6941b..8ebedef01 100644 --- a/test/storage/cache_revalidate.cpp +++ b/test/storage/cache_revalidate.cpp @@ -16,7 +16,8 @@ TEST_F(Storage, CacheRevalidate) { DefaultFileSource fs(&cache); const Resource revalidateSame { Resource::Unknown, "http://127.0.0.1:3000/revalidate-same" }; - fs.request(revalidateSame, uv_default_loop(), [&](const Response &res) { + Request* req = fs.request(revalidateSame, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Response", res.data); EXPECT_EQ(0, res.expires); @@ -24,7 +25,8 @@ TEST_F(Storage, CacheRevalidate) { EXPECT_EQ("snowfall", res.etag); EXPECT_EQ("", res.message); - fs.request(revalidateSame, uv_default_loop(), [&, res](const Response &res2) { + req = fs.request(revalidateSame, uv_default_loop(), [&, res](const Response &res2) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res2.status); EXPECT_EQ("Response", res2.data); // We use this to indicate that a 304 reply came back. @@ -40,7 +42,8 @@ TEST_F(Storage, CacheRevalidate) { const Resource revalidateModified{ Resource::Unknown, "http://127.0.0.1:3000/revalidate-modified" }; - fs.request(revalidateModified, uv_default_loop(), [&](const Response &res) { + Request* req2 = fs.request(revalidateModified, uv_default_loop(), [&](const Response &res) { + fs.cancel(req2); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Response", res.data); EXPECT_EQ(0, res.expires); @@ -48,7 +51,8 @@ TEST_F(Storage, CacheRevalidate) { EXPECT_EQ("", res.etag); EXPECT_EQ("", res.message); - fs.request(revalidateModified, uv_default_loop(), [&, res](const Response &res2) { + req2 = fs.request(revalidateModified, uv_default_loop(), [&, res](const Response &res2) { + fs.cancel(req2); EXPECT_EQ(Response::Successful, res2.status); EXPECT_EQ("Response", res2.data); // We use this to indicate that a 304 reply came back. @@ -62,7 +66,8 @@ TEST_F(Storage, CacheRevalidate) { }); const Resource revalidateEtag { Resource::Unknown, "http://127.0.0.1:3000/revalidate-etag" }; - fs.request(revalidateEtag, uv_default_loop(), [&](const Response &res) { + Request* req3 = fs.request(revalidateEtag, uv_default_loop(), [&](const Response &res) { + fs.cancel(req3); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Response 1", res.data); EXPECT_EQ(0, res.expires); @@ -70,7 +75,8 @@ TEST_F(Storage, CacheRevalidate) { EXPECT_EQ("response-1", res.etag); EXPECT_EQ("", res.message); - fs.request(revalidateEtag, uv_default_loop(), [&, res](const Response &res2) { + req3 = fs.request(revalidateEtag, uv_default_loop(), [&, res](const Response &res2) { + fs.cancel(req3); EXPECT_EQ(Response::Successful, res2.status); EXPECT_EQ("Response 2", res2.data); EXPECT_EQ(0, res2.expires); diff --git a/test/storage/database.cpp b/test/storage/database.cpp index b40474c00..20f96305b 100644 --- a/test/storage/database.cpp +++ b/test/storage/database.cpp @@ -181,7 +181,7 @@ TEST_F(Storage, DatabaseLockedWrite) { response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { - EXPECT_NE(nullptr, res.get()); + ASSERT_NE(nullptr, res.get()); EXPECT_EQ("Demo", res->data); }); @@ -258,7 +258,7 @@ TEST_F(Storage, DatabaseDeleted) { response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { - EXPECT_NE(nullptr, res.get()); + ASSERT_NE(nullptr, res.get()); EXPECT_EQ("Demo", res->data); }); @@ -275,7 +275,7 @@ TEST_F(Storage, DatabaseDeleted) { response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { - EXPECT_NE(nullptr, res.get()); + ASSERT_NE(nullptr, res.get()); EXPECT_EQ("Demo", res->data); }); @@ -305,7 +305,7 @@ TEST_F(Storage, DatabaseInvalid) { response->data = "Demo"; cache.put({ Resource::Unknown, "mapbox://test" }, response); cache.get({ Resource::Unknown, "mapbox://test" }, [] (std::unique_ptr res) { - EXPECT_NE(nullptr, res.get()); + ASSERT_NE(nullptr, res.get()); EXPECT_EQ("Demo", res->data); }); diff --git a/test/storage/directory_reading.cpp b/test/storage/directory_reading.cpp index 4459b42c2..11298dff8 100644 --- a/test/storage/directory_reading.cpp +++ b/test/storage/directory_reading.cpp @@ -15,8 +15,9 @@ TEST_F(Storage, AssetReadDirectory) { DefaultFileSource fs(nullptr); #endif - fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, uv_default_loop(), + Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Error, res.status); EXPECT_EQ(0ul, res.data.size()); EXPECT_EQ(0, res.expires); diff --git a/test/storage/file_reading.cpp b/test/storage/file_reading.cpp index 8db911f65..dc0620b78 100644 --- a/test/storage/file_reading.cpp +++ b/test/storage/file_reading.cpp @@ -16,8 +16,9 @@ TEST_F(Storage, AssetEmptyFile) { DefaultFileSource fs(nullptr); #endif - fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, uv_default_loop(), + Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/empty" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(0ul, res.data.size()); EXPECT_EQ(0, res.expires); @@ -41,8 +42,9 @@ TEST_F(Storage, AssetNonEmptyFile) { DefaultFileSource fs(nullptr); #endif - fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, + Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/nonempty" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(16ul, res.data.size()); EXPECT_EQ(0, res.expires); @@ -67,8 +69,9 @@ TEST_F(Storage, AssetNonExistentFile) { DefaultFileSource fs(nullptr); #endif - fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, + Request* req = fs.request({ Resource::Unknown, "asset://TEST_DATA/fixtures/storage/does_not_exist" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Error, res.status); EXPECT_EQ(0ul, res.data.size()); EXPECT_EQ(0, res.expires); diff --git a/test/storage/http_cancel.cpp b/test/storage/http_cancel.cpp index a73182da5..2eb599eea 100644 --- a/test/storage/http_cancel.cpp +++ b/test/storage/http_cancel.cpp @@ -36,7 +36,8 @@ TEST_F(Storage, HTTPCancelMultiple) { auto req2 = fs.request(resource, uv_default_loop(), [&](const Response &) { ADD_FAILURE() << "Callback should not be called"; }); - fs.request(resource, uv_default_loop(), [&](const Response &res) { + Request* req = fs.request(resource, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); diff --git a/test/storage/http_coalescing.cpp b/test/storage/http_coalescing.cpp index 5c90009f8..9dfa85550 100644 --- a/test/storage/http_coalescing.cpp +++ b/test/storage/http_coalescing.cpp @@ -40,8 +40,12 @@ TEST_F(Storage, HTTPCoalescing) { const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" }; + Request* reqs[total]; for (int i = 0; i < total; i++) { - fs.request(resource, uv_default_loop(), complete); + reqs[i] = fs.request(resource, uv_default_loop(), [&complete, &fs, &reqs, i] (const Response &res) { + fs.cancel(reqs[i]); + complete(res); + }); } uv_run(uv_default_loop(), UV_RUN_DEFAULT); diff --git a/test/storage/http_error.cpp b/test/storage/http_error.cpp index 7edce61e5..aebc75405 100644 --- a/test/storage/http_error.cpp +++ b/test/storage/http_error.cpp @@ -30,8 +30,9 @@ TEST_F(Storage, HTTPError) { auto start = uv_hrtime(); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, uv_default_loop(), + Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/temporary-error" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req1); const auto duration = double(uv_hrtime() - start) / 1e9; EXPECT_LT(1, duration) << "Backoff timer didn't wait 1 second"; EXPECT_GT(1.2, duration) << "Backoff timer fired too late"; @@ -45,8 +46,9 @@ TEST_F(Storage, HTTPError) { HTTPTemporaryError.finish(); }); - fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" }, uv_default_loop(), + Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3001/" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req2); const auto duration = double(uv_hrtime() - start) / 1e9; // 1.5 seconds == 4 retries, with a 500ms timeout (see above). EXPECT_LT(1.5, duration) << "Resource wasn't retried the correct number of times"; diff --git a/test/storage/http_header_parsing.cpp b/test/storage/http_header_parsing.cpp index 5f18b163a..13eab9912 100644 --- a/test/storage/http_header_parsing.cpp +++ b/test/storage/http_header_parsing.cpp @@ -15,9 +15,10 @@ TEST_F(Storage, HTTPHeaderParsing) { DefaultFileSource fs(nullptr); - fs.request({ Resource::Unknown, + Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?modified=1420794326&expires=1420797926&etag=foo" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req1); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(1420797926, res.expires); @@ -30,8 +31,9 @@ TEST_F(Storage, HTTPHeaderParsing) { int64_t now = std::chrono::duration_cast( SystemClock::now().time_since_epoch()).count(); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=120" }, + Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test?cachecontrol=max-age=120" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req2); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Hello World!", res.data); EXPECT_GT(2, std::abs(res.expires - now - 120)) << "Expiration date isn't about 120 seconds in the future"; diff --git a/test/storage/http_issue_1369.cpp b/test/storage/http_issue_1369.cpp index 44c190c82..954cb3b3f 100644 --- a/test/storage/http_issue_1369.cpp +++ b/test/storage/http_issue_1369.cpp @@ -30,7 +30,8 @@ TEST_F(Storage, HTTPIssue1369) { ADD_FAILURE() << "Callback should not be called"; }); fs.cancel(req); - fs.request(resource, uv_default_loop(), [&](const Response &res) { + req = fs.request(resource, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); diff --git a/test/storage/http_load.cpp b/test/storage/http_load.cpp index 2347a76ab..fa0468a84 100644 --- a/test/storage/http_load.cpp +++ b/test/storage/http_load.cpp @@ -15,11 +15,15 @@ TEST_F(Storage, HTTPLoad) { const int max = 10000; int number = 1; - std::function req = [&]() { + Request* reqs[concurrency]; + + std::function req = [&](int i) { const auto current = number++; - fs.request({ Resource::Unknown, + reqs[i] = fs.request({ Resource::Unknown, std::string("http://127.0.0.1:3000/load/") + std::to_string(current) }, - uv_default_loop(), [&, current](const Response &res) { + uv_default_loop(), [&, i, current](const Response &res) { + fs.cancel(reqs[i]); + reqs[i] = nullptr; EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ(std::string("Request ") + std::to_string(current), res.data); EXPECT_EQ(0, res.expires); @@ -28,7 +32,7 @@ TEST_F(Storage, HTTPLoad) { EXPECT_EQ("", res.message); if (number <= max) { - req(); + req(i); } else if (current == max) { HTTPLoad.finish(); } @@ -37,7 +41,7 @@ TEST_F(Storage, HTTPLoad) { for (int i = 0; i < concurrency; i++) { - req(); + req(i); } uv_run(uv_default_loop(), UV_RUN_DEFAULT); diff --git a/test/storage/http_other_loop.cpp b/test/storage/http_other_loop.cpp index 64612f13d..43a655fba 100644 --- a/test/storage/http_other_loop.cpp +++ b/test/storage/http_other_loop.cpp @@ -12,8 +12,9 @@ TEST_F(Storage, HTTPOtherLoop) { // This file source launches a separate thread to do the processing. DefaultFileSource fs(nullptr); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), + Request* req = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Hello World!", res.data); EXPECT_EQ(0, res.expires); diff --git a/test/storage/http_reading.cpp b/test/storage/http_reading.cpp index 78f69cae3..84af94302 100644 --- a/test/storage/http_reading.cpp +++ b/test/storage/http_reading.cpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -17,8 +18,9 @@ TEST_F(Storage, HTTPReading) { const auto mainThread = uv_thread_self(); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), + Request* req1 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req1); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::Successful, res.status); EXPECT_EQ("Hello World!", res.data); @@ -29,8 +31,9 @@ TEST_F(Storage, HTTPReading) { HTTPTest.finish(); }); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/doesnotexist" }, uv_default_loop(), + Request* req2 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/doesnotexist" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req2); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::NotFound, res.status); EXPECT_EQ("", res.message); @@ -40,8 +43,9 @@ TEST_F(Storage, HTTPReading) { HTTP404.finish(); }); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/permanent-error" }, uv_default_loop(), + Request* req3 = fs.request({ Resource::Unknown, "http://127.0.0.1:3000/permanent-error" }, uv_default_loop(), [&](const Response &res) { + fs.cancel(req3); EXPECT_EQ(uv_thread_self(), mainThread); EXPECT_EQ(Response::Error, res.status); EXPECT_EQ("HTTP status code 500", res.message); @@ -61,10 +65,14 @@ TEST_F(Storage, HTTPNoCallback) { DefaultFileSource fs(nullptr); - fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), + try { + fs.request({ Resource::Unknown, "http://127.0.0.1:3000/test" }, uv_default_loop(), nullptr); - - uv_run(uv_default_loop(), UV_RUN_DEFAULT); + } catch (const util::MisuseException& ex) { + EXPECT_EQ(std::string(ex.what()), "FileSource callback can't be empty"); + } catch (const std::exception&) { + EXPECT_TRUE(false) << "Unhandled exception."; + } HTTPTest.finish(); }