From 3944e1ae55c7be2e4cdd2b58b03964b121387469 Mon Sep 17 00:00:00 2001 From: Mykhailo Kuchma Date: Tue, 12 May 2020 17:49:43 +0300 Subject: [PATCH] TaskContext should not trigger cancellation. TaskContext should not trigger cancellation after request was completed. This could happen when the execution function keeps the last owning reference to the object that cancels task context in destruction. Resolves: OLPSUP-10456 Signed-off-by: Mykhailo Kuchma --- .../include/olp/core/client/TaskContext.h | 3 ++ .../tests/client/TaskContextTest.cpp | 38 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/olp-cpp-sdk-core/include/olp/core/client/TaskContext.h b/olp-cpp-sdk-core/include/olp/core/client/TaskContext.h index 6bf397ed1..a28b8ee9e 100644 --- a/olp-cpp-sdk-core/include/olp/core/client/TaskContext.h +++ b/olp-cpp-sdk-core/include/olp/core/client/TaskContext.h @@ -228,6 +228,9 @@ class CORE_API TaskContext { response.GetError().GetErrorCode() == ErrorCode::RequestTimeout)) { user_response = std::move(response); } + + // Reset the context after the task is finished. + context_ = CancellationContext(); } if (callback) { diff --git a/olp-cpp-sdk-core/tests/client/TaskContextTest.cpp b/olp-cpp-sdk-core/tests/client/TaskContextTest.cpp index 79a28f8f8..8ccc4d46f 100644 --- a/olp-cpp-sdk-core/tests/client/TaskContextTest.cpp +++ b/olp-cpp-sdk-core/tests/client/TaskContextTest.cpp @@ -216,4 +216,42 @@ TEST(TaskContextTest, CancelToken) { EXPECT_EQ(response.GetError().GetErrorCode(), ErrorCode::Cancelled); } +TEST(TaskContextTest, OLPSUP_10456) { + // Cancel should not be triggered from the inside of Execute function. + // This happens when the execution function is keeping the last owning + // reference to the object that cancels the task in the destructor. + struct TaskWrapper { + ~TaskWrapper() { context->BlockingCancel(std::chrono::milliseconds(0)); } + std::shared_ptr context; + }; + + auto wrapper = std::make_shared(); + + bool cancel_triggered = false; + auto cancel_function = [&]() { cancel_triggered = true; }; + + ExecuteFunc execute_func = [&, wrapper](CancellationContext c) -> Response { + c.ExecuteOrCancelled([&]() { return CancellationToken(cancel_function); }); + return std::string("Success"); + }; + + Response response; + Callback callback = [&](Response r) { response = std::move(r); }; + + // Initialize the task context + wrapper->context = std::make_shared( + TaskContext::Create(std::move(execute_func), std::move(callback))); + + TaskContext task_context = *(wrapper->context).get(); + + // Now execute_func is the only owner of task context via TaskWrapper. + wrapper.reset(); + + // Execute the execute_func, + task_context.Execute(); + + EXPECT_EQ(response.GetResult(), "Success"); + EXPECT_FALSE(cancel_triggered); +} + } // namespace