Skip to content

Commit bf61566

Browse files
authored
Add check for IsCancelled in NamedMutex (#1354)
Pass CancellationContext to NamedMutex. Check IsCancelled before locking mutex. Add GetBlobDataCancelParralellRequest test. Resolves: OLPSUP-20250 Signed-off-by: Yevhen Krasilnyk <[email protected]> Signed-off-by: Yevhen Krasilnyk <[email protected]>
1 parent 27030eb commit bf61566

File tree

7 files changed

+120
-19
lines changed

7 files changed

+120
-19
lines changed

olp-cpp-sdk-dataservice-read/src/ApiClientLookup.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2022 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -59,7 +59,8 @@ ApiClientLookup::ApiClientResponse ApiClientLookup::LookupApi(
5959
std::string service_version, FetchOptions options,
6060
client::OlpClientSettings settings, repository::NamedMutexStorage storage) {
6161
// This mutex is required to avoid concurrent requests to online.
62-
repository::NamedMutex mutex(storage, catalog.ToString());
62+
repository::NamedMutex mutex(storage, catalog.ToString(),
63+
cancellation_context);
6364
std::unique_lock<repository::NamedMutex> lock(mutex, std::defer_lock);
6465

6566
// If we are not planning to go online or access the cache, do not lock.

olp-cpp-sdk-dataservice-read/src/repositories/DataRepository.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2022 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -158,7 +158,8 @@ BlobApi::DataResponse DataRepository::GetBlobData(
158158
return client::ApiError::PreconditionFailed("Data handle is missing");
159159
}
160160

161-
NamedMutex mutex(storage_, catalog_.ToString() + layer + *data_handle);
161+
NamedMutex mutex(storage_, catalog_.ToString() + layer + *data_handle,
162+
context);
162163
std::unique_lock<NamedMutex> lock(mutex, std::defer_lock);
163164

164165
// If we are not planning to go online or access the cache, do not lock.

olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2020-2021 HERE Europe B.V.
2+
* Copyright (C) 2020-2022 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,8 @@
1919

2020
#include "NamedMutex.h"
2121

22+
#include <thread>
23+
2224
#include <olp/core/porting/make_unique.h>
2325

2426
namespace olp {
@@ -104,16 +106,31 @@ boost::optional<client::ApiError> NamedMutexStorage::GetError(
104106
return impl_->GetError(resource);
105107
}
106108

107-
NamedMutex::NamedMutex(NamedMutexStorage& storage, const std::string& name)
108-
: storage_{storage}, name_{name}, mutex_{storage_.AquireLock(name_)} {}
109+
NamedMutex::NamedMutex(NamedMutexStorage& storage, const std::string& name,
110+
const client::CancellationContext& context)
111+
: storage_{storage},
112+
context_{context},
113+
is_locked_{false},
114+
name_{name},
115+
mutex_{storage_.AquireLock(name_)} {}
109116

110117
NamedMutex::~NamedMutex() { storage_.ReleaseLock(name_); }
111118

112-
void NamedMutex::lock() { mutex_.lock(); }
119+
void NamedMutex::lock() {
120+
while (!is_locked_ && !context_.IsCancelled()) {
121+
is_locked_ = mutex_.try_lock();
122+
std::this_thread::yield();
123+
}
124+
}
113125

114-
bool NamedMutex::try_lock() { return mutex_.try_lock(); }
126+
bool NamedMutex::try_lock() { return is_locked_ = mutex_.try_lock(); }
115127

116-
void NamedMutex::unlock() { mutex_.unlock(); }
128+
void NamedMutex::unlock() {
129+
if (is_locked_) {
130+
mutex_.unlock();
131+
is_locked_ = false;
132+
}
133+
}
117134

118135
void NamedMutex::SetError(const client::ApiError& error) {
119136
storage_.SetError(name_, error);

olp-cpp-sdk-dataservice-read/src/repositories/NamedMutex.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2020-2021 HERE Europe B.V.
2+
* Copyright (C) 2020-2022 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
2525
#include <unordered_map>
2626

2727
#include <olp/core/client/ApiError.h>
28+
#include <olp/core/client/CancellationContext.h>
2829
#include <boost/optional.hpp>
2930

3031
namespace olp {
@@ -74,7 +75,8 @@ class NamedMutexStorage {
7475
*/
7576
class NamedMutex final {
7677
public:
77-
NamedMutex(NamedMutexStorage& storage, const std::string& name);
78+
NamedMutex(NamedMutexStorage& storage, const std::string& name,
79+
const client::CancellationContext& context);
7880

7981
NamedMutex(const NamedMutex&) = delete;
8082
NamedMutex(NamedMutex&&) = delete;
@@ -106,6 +108,8 @@ class NamedMutex final {
106108

107109
private:
108110
NamedMutexStorage& storage_;
111+
const client::CancellationContext& context_;
112+
bool is_locked_;
109113
std::string name_;
110114
std::mutex& mutex_;
111115
};

olp-cpp-sdk-dataservice-read/src/repositories/PartitionsRepository.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,8 @@ PartitionsRepository::GetPartitionsExtendedResponse(
242242
partition_ids.empty() ? "" : HashPartitions(partition_ids);
243243
const auto version_str = version ? std::to_string(*version) : "";
244244

245-
NamedMutex mutex(storage_, catalog_str + layer_id_ + version_str + detail);
245+
NamedMutex mutex(storage_, catalog_str + layer_id_ + version_str + detail,
246+
context);
246247
std::unique_lock<NamedMutex> lock(mutex, std::defer_lock);
247248

248249
// If we are not planning to go online or access the cache, do not lock.
@@ -343,7 +344,7 @@ PartitionsResponse PartitionsRepository::GetPartitionById(
343344
const auto request_key =
344345
catalog_.ToString() + request.CreateKey(layer_id_, version);
345346

346-
NamedMutex mutex(storage_, request_key);
347+
NamedMutex mutex(storage_, request_key, context);
347348
std::unique_lock<repository::NamedMutex> lock(mutex, std::defer_lock);
348349

349350
// If we are not planning to go online or access the cache, do not lock.
@@ -431,7 +432,7 @@ QuadTreeIndexResponse PartitionsRepository::GetQuadTreeIndexForTile(
431432
catalog_.ToCatalogHRNString(), layer_id_, root_tile_key, version,
432433
kAggregateQuadTreeDepth);
433434

434-
NamedMutex mutex(storage_, quad_cache_key);
435+
NamedMutex mutex(storage_, quad_cache_key, context);
435436
std::unique_lock<NamedMutex> lock(mutex, std::defer_lock);
436437

437438
// If we are not planning to go online or access the cache, do not lock.

olp-cpp-sdk-dataservice-read/src/repositories/PrefetchTilesRepository.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ client::NetworkStatistics PrefetchTilesRepository::LoadAggregatedSubQuads(
201201
const auto quad_cache_key = cache::KeyGenerator::CreateQuadTreeKey(
202202
catalog_str_, layer_id_, root, version, kMaxQuadTreeIndexDepth);
203203

204-
NamedMutex mutex(storage_, quad_cache_key);
204+
NamedMutex mutex(storage_, quad_cache_key, context);
205205
std::unique_lock<NamedMutex> lock(mutex);
206206

207207
if (!cache_repository_.ContainsTree(root, kMaxQuadTreeIndexDepth,
@@ -229,7 +229,7 @@ SubQuadsResponse PrefetchTilesRepository::GetVersionedSubQuads(
229229
const auto quad_cache_key = cache::KeyGenerator::CreateQuadTreeKey(
230230
catalog_str_, layer_id_, tile, version, kMaxQuadTreeIndexDepth);
231231

232-
NamedMutex mutex(storage_, quad_cache_key);
232+
NamedMutex mutex(storage_, quad_cache_key, context);
233233
std::lock_guard<NamedMutex> lock(mutex);
234234

235235
if (cache_repository_.Get(tile, depth, version, quad_tree)) {

olp-cpp-sdk-dataservice-read/tests/DataRepositoryTest.cpp

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2019-2021 HERE Europe B.V.
2+
* Copyright (C) 2019-2022 HERE Europe B.V.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,10 @@
1919

2020
#include <gtest/gtest.h>
2121

22+
#include <chrono>
23+
#include <future>
24+
#include <thread>
25+
2226
#include <matchers/NetworkUrlMatchers.h>
2327
#include <mocks/NetworkMock.h>
2428
#include <olp/core/cache/CacheSettings.h>
@@ -321,7 +325,7 @@ TEST_F(DataRepositoryTest, GetBlobDataSimultaniousFailedCalls) {
321325
// accuares the mutex, the stored error will not be cleaned up in scope of
322326
// ReleaseLock call from the first thread
323327
olp::dataservice::read::repository::NamedMutex mutex(
324-
storage, hrn.ToString() + kService + kUrlBlobDataHandle);
328+
storage, hrn.ToString() + kService + kUrlBlobDataHandle, context);
325329

326330
// Start second request in a separate thread
327331
std::thread second_request_thread([&]() {
@@ -523,4 +527,77 @@ TEST_F(DataRepositoryTest, GetVersionedDataTileReturnEmpty) {
523527
ASSERT_EQ(response.GetError().GetMessage(),
524528
"Failed to parse quad tree response");
525529
}
530+
531+
TEST_F(DataRepositoryTest, GetBlobDataCancelParralellRequest) {
532+
EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlLookup), _, _, _, _))
533+
.WillRepeatedly(
534+
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
535+
olp::http::HttpStatusCode::OK),
536+
kUrlResponseLookup));
537+
538+
constexpr auto wait_time = std::chrono::seconds(1);
539+
auto wait = [&wait_time]() { std::this_thread::sleep_for(wait_time); };
540+
541+
EXPECT_CALL(*network_mock_, Send(IsGetRequest(kUrlBlobData269), _, _, _, _))
542+
.WillRepeatedly(testing::DoAll(
543+
testing::InvokeWithoutArgs(wait),
544+
ReturnHttpResponse(olp::http::NetworkResponse().WithStatus(
545+
olp::http::HttpStatusCode::OK),
546+
"Some Data")));
547+
548+
olp::client::CancellationContext context;
549+
olp::dataservice::read::repository::NamedMutexStorage storage;
550+
551+
olp::dataservice::read::DataRequest request;
552+
request.WithDataHandle(kUrlBlobDataHandle);
553+
554+
olp::client::HRN hrn(GetTestCatalog());
555+
ApiLookupClient lookup_client(hrn, *settings_);
556+
DataRepository repository(hrn, *settings_, lookup_client, storage);
557+
558+
std::promise<void> first_thread_finished_promise;
559+
std::promise<void> second_thread_finished_promise;
560+
561+
// Start first request in a separate thread
562+
std::thread first_request_thread([&]() {
563+
auto response =
564+
repository.GetBlobData(kLayerId, kService, request, context);
565+
566+
EXPECT_FALSE(response);
567+
EXPECT_EQ(response.GetError().GetErrorCode(),
568+
olp::client::ErrorCode::Cancelled);
569+
570+
first_thread_finished_promise.set_value();
571+
});
572+
573+
// Start second request in a separate thread
574+
std::thread second_request_thread([&]() {
575+
auto response =
576+
repository.GetBlobData(kLayerId, kService, request, context);
577+
578+
EXPECT_FALSE(response);
579+
EXPECT_EQ(response.GetError().GetErrorCode(),
580+
olp::client::ErrorCode::Cancelled);
581+
582+
second_thread_finished_promise.set_value();
583+
});
584+
585+
const auto start = std::chrono::high_resolution_clock::now();
586+
587+
// Start threads w/o wait for them to finish
588+
first_request_thread.detach();
589+
second_request_thread.detach();
590+
591+
// Cancel operation should immeadeately finish both requests
592+
context.CancelOperation();
593+
594+
// Wait until threads are finished
595+
first_thread_finished_promise.get_future().wait();
596+
second_thread_finished_promise.get_future().wait();
597+
598+
const auto end = std::chrono::high_resolution_clock::now();
599+
600+
// Compare time spanding for waiting for threads to finish
601+
EXPECT_LT(end - start, wait_time);
602+
}
526603
} // namespace

0 commit comments

Comments
 (0)