Skip to content

Commit b1cd55b

Browse files
authored
Add TaskContinuation tests/change cancellation (#1307)
Add unit tests that check asynchronous behaviour of TaskContinuation. Add clearing a callback and tasks on cancellation. Fix calling SetError in case a continuation is cancelled. Change the mechanism to cover cancellation cases(cancel before/ after run) in case of calling SetError. Simplify usage of promise/future. Relates-To: OLPEDGE-2721 Signed-off-by: Yevhenii Dudnyk <[email protected]>
1 parent 4825262 commit b1cd55b

File tree

6 files changed

+492
-243
lines changed

6 files changed

+492
-243
lines changed

olp-cpp-sdk-core/include/olp/core/thread/Continuation.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,6 @@ class CORE_API ContinuationImpl final {
9999
*/
100100
const ExecutionContext& GetExecutionContext() const;
101101

102-
/**
103-
* @brief Gets the `CancellationContext` object.
104-
*
105-
* @return The `CancellationContext` instance.
106-
*/
107-
const client::CancellationContext& GetContext() const;
108-
109102
/**
110103
* @brief Checks whether the `CancellationContext` instance is cancelled.
111104
*

olp-cpp-sdk-core/include/olp/core/thread/Continuation.inl

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -29,29 +29,48 @@ class TaskScheduler;
2929

3030
template <typename ResultType>
3131
void Continuation<ResultType>::Run() {
32+
// Run should not start an execution if the `Finally` method is not called
33+
// for the task continuation.
3234
if (!finally_callback_) {
3335
impl_.Clear();
3436
return;
3537
}
3638

37-
impl_.SetFailedCallback(
38-
[this](client::ApiError error) { finally_callback_(std::move(error)); });
39-
39+
// In case of the continuation is cancelled before calling the `Run` method,
40+
// return the `Cancelled` error.
4041
if (impl_.Cancelled()) {
41-
finally_callback_(client::ApiError::Cancelled());
42+
const auto finally_callback = std::move(finally_callback_);
43+
if (finally_callback) {
44+
finally_callback(client::ApiError::Cancelled());
45+
}
46+
4247
impl_.Clear();
43-
finally_callback_ = nullptr;
4448
return;
4549
}
4650

47-
impl_.Run([this](void* input, bool cancelled) {
48-
if (cancelled) {
49-
finally_callback_(client::ApiError::Cancelled());
50-
} else {
51-
auto value = std::move(*static_cast<ResultType*>(input));
52-
finally_callback_(std::move(value));
51+
// Use std::move to set the `finally_callback_` member to nullptr.
52+
// It's needed to check if the execution is done, and prevent calling
53+
// the `Cancel` method of the `Continuation` object.
54+
// The `SetError` method of the `ExecutionContext` should not execute a
55+
// callback as well.
56+
impl_.SetFailedCallback([=](client::ApiError error) {
57+
const auto finally_callback = std::move(finally_callback_);
58+
if (finally_callback) {
59+
finally_callback(std::move(error));
60+
}
61+
});
62+
63+
impl_.Run([=](void* input, bool cancelled) {
64+
impl_.Clear();
65+
66+
const auto finally_callback = std::move(finally_callback_);
67+
if (finally_callback) {
68+
if (cancelled) {
69+
finally_callback(client::ApiError::Cancelled());
70+
} else {
71+
finally_callback(std::move(*static_cast<ResultType*>(input)));
72+
}
5373
}
54-
finally_callback_ = nullptr;
5574
});
5675
}
5776

@@ -104,21 +123,19 @@ Continuation<ResultType>::Then(std::function<void(ExecutionContext, ResultType,
104123
std::function<void(NewType)>)>
105124
task) {
106125
using NewResultType = internal::RemoveRefAndConst<NewType>;
107-
auto context = impl_.GetExecutionContext();
108-
109-
return impl_.Then({[=](void* input, CallbackType callback) {
110-
auto in = *static_cast<ResultType*>(input);
111-
task(std::move(context), std::move(in),
112-
[callback, context](NewResultType arg) {
113-
callback(static_cast<void*>(&arg));
114-
});
115-
},
116-
[](void* input) {
117-
auto in = *static_cast<NewResultType*>(input);
118-
auto result =
119-
std::make_unique<NewResultType>(std::move(in));
120-
return internal::MakeUntypedUnique(std::move(result));
121-
}});
126+
const auto context = impl_.GetExecutionContext();
127+
128+
return impl_.Then(
129+
{[=](void* input, CallbackType callback) {
130+
auto in = *static_cast<ResultType*>(input);
131+
task(std::move(context), std::move(in),
132+
[=](NewResultType arg) { callback(static_cast<void*>(&arg)); });
133+
},
134+
[](void* input) {
135+
auto in = *static_cast<NewResultType*>(input);
136+
auto result = std::make_unique<NewResultType>(std::move(in));
137+
return internal::MakeUntypedUnique(std::move(result));
138+
}});
122139
}
123140

124141
template <typename ResultType>
@@ -127,9 +144,9 @@ client::CancellationToken Continuation<ResultType>::CancelToken() {
127144
return client::CancellationToken();
128145
}
129146

130-
auto context = impl_.GetContext();
147+
auto context = impl_.GetExecutionContext();
131148
return client::CancellationToken(
132-
[context]() mutable { context.CancelOperation(); });
149+
[=]() mutable { context.CancelOperation(); });
133150
}
134151

135152
template <typename ResultType>

olp-cpp-sdk-core/src/thread/Continuation.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ class Processor {
114114

115115
// creates a task for execution by TaskContinuation
116116
Task CreateFollowingTask(const std::shared_ptr<ProcessorInternal>& context) {
117-
return [this, context] { ProcessNextTask(context); };
117+
return [=] { ProcessNextTask(context); };
118118
}
119119

120120
// starts the first task in a chain
@@ -148,10 +148,6 @@ class Processor {
148148
// checks whether the Continuation is cancelled
149149
bool IsCancelled() const { return public_execution_context_.Cancelled(); }
150150

151-
const ExecutionContext& GetExecutionContext() {
152-
return public_execution_context_;
153-
}
154-
155151
// An input argument used by the current task in the queue, it's the result
156152
// of the previous task
157153
void* LastOutput() const {
@@ -193,10 +189,6 @@ const ExecutionContext& ContinuationImpl::GetExecutionContext() const {
193189
return execution_context_;
194190
}
195191

196-
const client::CancellationContext& ContinuationImpl::GetContext() const {
197-
return execution_context_.GetContext();
198-
}
199-
200192
bool ContinuationImpl::Cancelled() const {
201193
return execution_context_.Cancelled();
202194
}

0 commit comments

Comments
 (0)