Skip to content

Commit

Permalink
TaskContext should not trigger cancellation. (#833)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mykhailo-kuchma authored May 13, 2020
1 parent f8437a8 commit 311adc8
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
3 changes: 3 additions & 0 deletions olp-cpp-sdk-core/include/olp/core/client/TaskContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
38 changes: 38 additions & 0 deletions olp-cpp-sdk-core/tests/client/TaskContextTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<TaskContext> context;
};

auto wrapper = std::make_shared<TaskWrapper>();

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>(
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

0 comments on commit 311adc8

Please sign in to comment.