You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by pn...@apache.org on 2021/10/05 15:52:43 UTC
[celix] 01/01: Updates Promise `resolve` and `fail` so make them
robust.
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/robust_promise_fail_and_resolve
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 43296771b1fc77fbf83fc4f1ffa4380331542900
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Tue Oct 5 17:52:26 2021 +0200
Updates Promise `resolve` and `fail` so make them robust.
Also add a `tryResolve` and `tryFail` alternative to be able to resolve a promise
and check if it was already resolved.
---
libs/promises/README.md | 11 +-
libs/promises/api/celix/Deferred.h | 181 +++++++++++++++++----
libs/promises/api/celix/Promise.h | 10 +-
libs/promises/api/celix/impl/SharedPromiseState.h | 184 ++++++++--------------
libs/promises/gtest/src/PromisesTestSuite.cc | 123 ++++++++++++++-
libs/promises/gtest/src/VoidPromisesTestSuite.cc | 102 ++++++++++++
6 files changed, 454 insertions(+), 157 deletions(-)
diff --git a/libs/promises/README.md b/libs/promises/README.md
index 3197cec..dd7f1bd 100644
--- a/libs/promises/README.md
+++ b/libs/promises/README.md
@@ -78,11 +78,16 @@ target_link_libraries(PromiseExamples PRIVATE Celix::Promises)
3. The default constructor for celix::Deferred has been removed. A celix:Deferred can only be created through a PromiseFactory. This is done because the promise concept is heavily bound with the execution abstraction and thus a execution model. Creating a Deferred without a explicit executor is not desirable.
4. The PromiseFactory also has a deferredTask method. This is a convenient method create a Deferred, execute a task async to resolve the Deferred and return a Promise of the created Deferred in one call.
5. The celix::IExecutor abstraction has a priority argument (and as result also the calls in PromiseFactory, etc).
-6. The IExecutor has a added wait() method. This can be used to ensure a executor is done executing the tasks backlog.
-
-
+6. The IExecutor has a added wait() method. This can be used to ensure an executor is done executing the tasks backlog.
+7. The `celix::Deferred<T>::fail` and `celix::Deferred<T>::resolve` are make robust for resolving a
+ deferred if the associated promise is already resolved. This is different from the OSGi spec,
+ because it always a race condition to check if a promise is already resolved (`isDone()`)
+ and then resolve the deferred. The methods `celix::Deferred<T>::tryFail` and
+ `celix::Deferred<T>::tryResolve` exist to resolve a deferred and check if it was
+ already resolved atomically.
## Open Issues & TODOs
+
- Documentation not complete
- PromiseFactory is not complete yet
- The static helper class Promises is not implemented yet (e.g. all/any)
diff --git a/libs/promises/api/celix/Deferred.h b/libs/promises/api/celix/Deferred.h
index bf8f1ff..eb07091 100644
--- a/libs/promises/api/celix/Deferred.h
+++ b/libs/promises/api/celix/Deferred.h
@@ -53,7 +53,8 @@ namespace celix {
explicit Deferred(std::shared_ptr<celix::impl::SharedPromiseState<T>> state);
/**
- * Fail the Promise associated with this Deferred.
+ * @brief Fail the Promise associated with this Deferred.
+ *
* <p/>
* After the associated Promise is resolved with the specified failure, all registered callbacks are called and any
* chained Promises are resolved.
@@ -61,13 +62,23 @@ namespace celix {
* Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
* callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
*
+ * If the associated promise is already resolved, the call will be ignored.
+ *
* @param failure The failure in the form of an exception pointer.
- * @throws PromiseInvocationException If the associated Promise was already resolved.
*/
void fail(std::exception_ptr failure);
/**
- * Fail the Promise associated with this Deferred.
+ * @brief Try to fail the Promise associated with this Deferred.
+ *
+ * Same as `fail`, but will return `true` if the associated promise was successfully failed and `false` if
+ * the associated promise was already resolved.
+ */
+ bool tryFail(std::exception_ptr failure);
+
+ /**
+ * @brief Fail the Promise associated with this Deferred.
+ *
* <p/>
* After the associated Promise is resolved with the specified failure, all registered callbacks are called and any
* chained Promises are resolved.
@@ -75,24 +86,38 @@ namespace celix {
* Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
* callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
*
+ * If the associated promise is already resolved, the call will be ignored.
+ *
* @param failure The failure in the form of an const std::exception reference.
- * @throws PromiseInvocationException If the associated Promise was already resolved.
*/
- template<typename E, typename std::enable_if_t< std::is_base_of<std::exception, E>::value, bool> = true >
+ template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool> = true >
void fail(const E& failure);
+
/**
- * Returns the Promise associated with this Deferred.
+ * @brief Try to fail the Promise associated with this Deferred.
+ *
+ * Same as `fail`, but will return `true` if the associated promise was successfully failed and `false` if
+ * the associated promise was already resolved.
+ */
+ template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool> = true >
+ bool tryFail(const E& failure);
+
+ /**
+ * @brief Returns the Promise associated with this Deferred.
+ *
* <p>
* All Promise objects created by the associated Promise will use the
* executors of the associated Promise.
+ * </p>
*
* @return The Promise associated with this Deferred.
*/
[[nodiscard]] Promise<T> getPromise();
/**
- * Successfully resolve the Promise associated with this Deferred.
+ * @brief Resolve the Promise associated with this Deferred.
+ *
* <p/>
* After the associated Promise is resolved with the specified value, all registered callbacks are called and any
* chained Promises are resolved.
@@ -100,14 +125,47 @@ namespace celix {
* Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
* callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
*
+ * If the associated promise is already resolved, the call will be ignored.
+ *
* @param value The value of the resolved Promise.
- * @throws PromiseInvocationException If the associated Promise was already resolved.
*/
void resolve(T&& value);
+
+ /**
+ * @brief Resolve the Promise associated with this Deferred.
+ *
+ * <p/>
+ * After the associated Promise is resolved with the specified value, all registered callbacks are called and any
+ * chained Promises are resolved.
+ * <p/>
+ * Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
+ * callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
+ *
+ * If the associated promise is already resolved, the call will be ignored.
+ *
+ * @param value The value of the resolved Promise.
+ */
void resolve(const T& value);
/**
- * Resolve the Promise associated with this Deferred with the specified Promise.
+ * @brief Try to resolve the Promise associated with this Deferred.
+ *
+ * Same as `resolve`, but will return `true` if the associated promise was successfully resolved and `false` if
+ * the associated promise was already resolved.
+ */
+ bool tryResolve(T&& value);
+
+ /**
+ * @brief Try to resolve the Promise associated with this Deferred.
+ *
+ * Same as `resolve`, but will return `true` if the associated promise was successfully resolved and `false` if
+ * the associated promise was already resolved.
+ */
+ bool tryResolve(const T& value);
+
+ /**
+ * @brief Resolve the Promise associated with this Deferred with the specified Promise.
+ *
* <p/>
* If the specified Promise is successfully resolved, the associated Promise is resolved with the value of the
* specified Promise. If the specified Promise is resolved with a failure, the associated Promise is resolved with
@@ -140,10 +198,9 @@ namespace celix {
explicit Deferred(std::shared_ptr<celix::impl::SharedPromiseState<void>> state);
- //TODO deferred ctor with factory
-
/**
- * Fail the Promise associated with this Deferred.
+ * @brief Fail the Promise associated with this Deferred.
+ *
* <p/>
* After the associated Promise is resolved with the specified failure, all registered callbacks are called and any
* chained Promises are resolved.
@@ -151,13 +208,23 @@ namespace celix {
* Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
* callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
*
+ * If the associated promise is already resolved, the call will be ignored.
+ *
* @param failure The failure in the form of an exception pointer.
- * @throws PromiseInvocationException If the associated Promise was already resolved.
*/
void fail(std::exception_ptr failure);
/**
- * Fail the Promise associated with this Deferred.
+ * @brief Try to fail the Promise associated with this Deferred.
+ *
+ * Same as `fail`, but will return `true` if the associated promise was successfully failed and `false` if
+ * the associated promise was already resolved.
+ */
+ bool tryFail(std::exception_ptr failure);
+
+ /**
+ * @brief Fail the Promise associated with this Deferred.
+ *
* <p/>
* After the associated Promise is resolved with the specified failure, all registered callbacks are called and any
* chained Promises are resolved.
@@ -165,24 +232,37 @@ namespace celix {
* Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
* callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
*
+ * If the associated promise is already resolved, the call will be ignored.
+ *
* @param failure The failure in the form of an const std::exception reference.
- * @throws PromiseInvocationException If the associated Promise was already resolved.
*/
- template<typename E, typename std::enable_if_t< std::is_base_of<std::exception, E>::value, bool> = true >
+ template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool> = true >
void fail(const E& failure);
/**
- * Returns the Promise associated with this Deferred.
+ * @brief Try to fail the Promise associated with this Deferred.
+ *
+ * Same as `fail`, but will return `true` if the associated promise was successfully failed and `false` if
+ * the associated promise was already resolved.
+ */
+ template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool> = true >
+ bool tryFail(const E& failure);
+
+ /**
+ * @brief Returns the Promise associated with this Deferred.
+ *
* <p>
* All Promise objects created by the associated Promise will use the
* executors of the associated Promise.
+ * </p>
*
* @return The Promise associated with this Deferred.
*/
[[nodiscard]] Promise<void> getPromise();
/**
- * Successfully resolve the Promise associated with this Deferred.
+ * @brief Resolve the Promise associated with this Deferred.
+ *
* <p/>
* After the associated Promise is resolved with the specified value, all registered callbacks are called and any
* chained Promises are resolved.
@@ -190,11 +270,20 @@ namespace celix {
* Resolving the associated Promise happens-before any registered callback is called. That is, in a registered
* callback, Promise.isDone() must return true and Promise.getValue() and Promise.getFailure() must not block.
*
+ * If the associated promise is already resolved, the call will be ignored.
+ *
* @param value The value of the resolved Promise.
- * @throws PromiseInvocationException If the associated Promise was already resolved.
*/
void resolve();
+ /**
+ * @brief Try to resolve the Promise associated with this Deferred.
+ *
+ * Same as `resolve`, but will return `true` if the associated promise was successfully resolved and `false` if
+ * the associated promise was already resolved.
+ */
+ bool tryResolve();
+
template<typename U>
celix::Promise<void> resolveWith(celix::Promise<U> with);
private:
@@ -215,22 +304,42 @@ inline celix::Deferred<void>::Deferred(std::shared_ptr<celix::impl::SharedPromis
template<typename T>
void celix::Deferred<T>::fail(std::exception_ptr failure) {
- state->fail(std::move(failure));
+ state->tryFail(std::move(failure));
+}
+
+template<typename T>
+bool celix::Deferred<T>::tryFail(std::exception_ptr failure) {
+ return state->tryFail(std::move(failure));
}
inline void celix::Deferred<void>::fail(std::exception_ptr failure) {
- state->fail(std::move(failure));
+ state->tryFail(std::move(failure));
+}
+
+inline bool celix::Deferred<void>::tryFail(std::exception_ptr failure) {
+ return state->tryFail(std::move(failure));
}
template<typename T>
-template<typename E, typename std::enable_if_t< std::is_base_of<std::exception, E>::value, bool>>
+template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool>>
void celix::Deferred<T>::fail(const E& failure) {
- state->template fail<E>(failure);
+ state->template tryFail<E>(failure);
+}
+
+template<typename T>
+template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool>>
+bool celix::Deferred<T>::tryFail(const E& failure) {
+ return state->template tryFail<E>(failure);
}
-template<typename E, typename std::enable_if_t< std::is_base_of<std::exception, E>::value, bool>>
-inline void celix::Deferred<void>::fail(const E& failure) {
- state->template fail<E>(failure);
+template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool>>
+void celix::Deferred<void>::fail(const E& failure) {
+ state->tryFail<E>(failure);
+}
+
+template<typename E, typename std::enable_if_t< std::is_base_of_v<std::exception, E>, bool>>
+bool celix::Deferred<void>::tryFail(const E& failure) {
+ return state->tryFail<E>(failure);
}
template<typename T>
@@ -286,14 +395,28 @@ inline celix::Promise<void> celix::Deferred<void>::resolveWith(celix::Promise<U>
template<typename T>
void celix::Deferred<T>::resolve(T&& value) {
- state->resolve(std::forward<T>(value));
+ state->tryResolve(std::forward<T>(value));
}
template<typename T>
void celix::Deferred<T>::resolve(const T& value) {
- state->resolve(value);
+ state->tryResolve(value);
}
inline void celix::Deferred<void>::resolve() {
- state->resolve();
+ state->tryResolve();
+}
+
+template<typename T>
+bool celix::Deferred<T>::tryResolve(T&& value) {
+ return state->tryResolve(std::forward<T>(value));
}
+
+template<typename T>
+bool celix::Deferred<T>::tryResolve(const T& value) {
+ return state->tryResolve(value);
+}
+
+inline bool celix::Deferred<void>::tryResolve() {
+ return state->tryResolve();
+}
\ No newline at end of file
diff --git a/libs/promises/api/celix/Promise.h b/libs/promises/api/celix/Promise.h
index 1ce6b32..3d1312d 100644
--- a/libs/promises/api/celix/Promise.h
+++ b/libs/promises/api/celix/Promise.h
@@ -745,14 +745,13 @@ inline celix::Promise<U> celix::Promise<T>::then(std::function<celix::Promise<U>
auto tmpPromise = success(celix::Promise<T>{s});
p->resolveWith(*tmpPromise.state);
} catch (...) {
- //failure(); TODO not sure if this needs to be called
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
} else {
if (failure) {
failure(celix::Promise<T>{s});
}
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
};
state->addChain(std::move(chain));
@@ -770,14 +769,13 @@ inline celix::Promise<U> celix::Promise<void>::then(std::function<celix::Promise
auto tmpPromise = success(celix::Promise<void>{s});
p->resolveWith(*tmpPromise.state);
} catch (...) {
- //failure(); TODO not sure if this needs to be called
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
} else {
if (failure) {
failure(celix::Promise<void>{s});
}
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
};
state->addChain(std::move(chain));
diff --git a/libs/promises/api/celix/impl/SharedPromiseState.h b/libs/promises/api/celix/impl/SharedPromiseState.h
index ebe39d7..f92fa05 100644
--- a/libs/promises/api/celix/impl/SharedPromiseState.h
+++ b/libs/promises/api/celix/impl/SharedPromiseState.h
@@ -46,22 +46,18 @@ namespace celix::impl {
~SharedPromiseState() noexcept = default;
- void resolve(T&& value);
-
- void resolve(const T& value);
-
template<typename U>
void resolveWith(SharedPromiseState<U>& with);
- void fail(std::exception_ptr e);
-
- template<typename E>
- void fail(const E &e);
+ bool tryResolve(T&& value);
- bool tryResolve(T &&value);
+ bool tryResolve(const T& value);
bool tryFail(std::exception_ptr e);
+ template<typename E>
+ bool tryFail(const E& e);
+
// copy/move depending on situation
T& getValue() &;
const T& getValue() const &;
@@ -151,17 +147,13 @@ namespace celix::impl {
~SharedPromiseState() noexcept = default;
- void resolve();
-
- void fail(std::exception_ptr e);
-
- template<typename E>
- void fail(const E &e);
-
bool tryResolve();
bool tryFail(std::exception_ptr e);
+ template<typename E>
+ bool tryFail(const E& e);
+
bool getValue() const;
std::exception_ptr getFailure() const;
@@ -278,79 +270,28 @@ inline std::weak_ptr<celix::impl::SharedPromiseState<void>> celix::impl::SharedP
}
template<typename T>
-void celix::impl::SharedPromiseState<T>::resolve(T&& value) {
- std::unique_lock<std::mutex> lck{mutex};
- if (done) {
- throw celix::PromiseInvocationException("Cannot resolve Promise. Promise is already done");
- }
- dataMoved = false;
- if constexpr (std::is_move_constructible_v<T>) {
- data = std::forward<T>(value);
- } else {
- data = value;
- }
- exp = nullptr;
- complete(lck);
-}
-
-
-template<typename T>
-void celix::impl::SharedPromiseState<T>::resolve(const T& value) {
- std::unique_lock<std::mutex> lck{mutex};
- if (done) {
- throw celix::PromiseInvocationException("Cannot resolve Promise. Promise is already done");
- }
- dataMoved = false;
- data = value;
- exp = nullptr;
- complete(lck);
-}
-
-inline void celix::impl::SharedPromiseState<void>::resolve() {
- std::unique_lock<std::mutex> lck{mutex};
- if (done) {
- throw celix::PromiseInvocationException("Cannot resolve Promise. Promise is already done");
- }
- exp = nullptr;
- complete(lck);
-}
-
-template<typename T>
-void celix::impl::SharedPromiseState<T>::fail(std::exception_ptr e) {
- std::unique_lock<std::mutex> lck{mutex};
- if (done) {
- throw celix::PromiseInvocationException("Cannot fail Promise. Promise is already done");
- }
- exp = std::move(e);
- complete(lck);
-}
-
-inline void celix::impl::SharedPromiseState<void>::fail(std::exception_ptr e) {
+bool celix::impl::SharedPromiseState<T>::tryResolve(T&& value) {
std::unique_lock<std::mutex> lck{mutex};
- if (done) {
- throw celix::PromiseInvocationException("Cannot fail Promise. Promise is already done");
+ if (!done) {
+ dataMoved = false;
+ if constexpr (std::is_move_constructible_v<T>) {
+ data = std::forward<T>(value);
+ } else {
+ data = value;
+ }
+ exp = nullptr;
+ complete(lck);
+ return true;
}
- exp = std::move(e);
- complete(lck);
-}
-
-template<typename T>
-template<typename E>
-void celix::impl::SharedPromiseState<T>::fail(const E& e) {
- fail(std::make_exception_ptr(e));
-}
-
-template<typename E>
-inline void celix::impl::SharedPromiseState<void>::fail(const E& e) {
- fail(std::make_exception_ptr<E>(e));
+ return false;
}
template<typename T>
-bool celix::impl::SharedPromiseState<T>::tryResolve(T&& value) {
+bool celix::impl::SharedPromiseState<T>::tryResolve(const T& value) {
std::unique_lock<std::mutex> lck{mutex};
if (!done) {
dataMoved = false;
- data = std::forward<T>(value);
+ data = value;
exp = nullptr;
complete(lck);
return true;
@@ -390,6 +331,17 @@ inline bool celix::impl::SharedPromiseState<void>::tryFail(std::exception_ptr e)
}
template<typename T>
+template<typename E>
+bool celix::impl::SharedPromiseState<T>::tryFail(const E& e) {
+ return tryFail(std::make_exception_ptr<E>(e));
+}
+
+template<typename E>
+bool celix::impl::SharedPromiseState<void>::tryFail(const E& e) {
+ return tryFail(std::make_exception_ptr<E>(e));
+}
+
+template<typename T>
bool celix::impl::SharedPromiseState<T>::isDone() const {
std::lock_guard lck{mutex};
return done;
@@ -623,14 +575,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<T>> celix::impl::SharedPromiseSt
state->scheduledExecutor->schedule(state->priority, duration, [v = std::move(v), e, state] {
try {
if (v) {
- state->resolve(std::move(*v));
+ state->tryResolve(std::move(*v));
} else {
- state->fail(e);
+ state->tryFail(e);
}
- } catch (celix::PromiseInvocationException &) {
- //somebody already resolved promise?
} catch (...) {
- state->fail(std::current_exception());
+ state->tryFail(std::current_exception());
}
});
});
@@ -644,14 +594,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<void>> celix::impl::SharedPromis
state->scheduledExecutor->schedule(state->priority, duration, [e, state] {
try {
if (!e) {
- state->resolve();
+ state->tryResolve();
} else {
- state->fail(*e);
+ state->tryFail(*e);
}
- } catch (celix::PromiseInvocationException &) {
- //somebody already resolved promise?
} catch (...) {
- state->fail(std::current_exception());
+ state->tryFail(std::current_exception());
}
});
});
@@ -666,12 +614,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<T>> celix::impl::SharedPromiseSt
auto p = celix::impl::SharedPromiseState<T>::create(executor, scheduledExecutor, priority);
addOnResolve([p, recover = std::move(recover)](std::optional<T> v, const std::exception_ptr& /*e*/) {
if (v) {
- p->resolve(std::move(*v));
+ p->tryResolve(std::move(*v));
} else {
try {
- p->resolve(recover());
+ p->tryResolve(recover());
} catch (...) {
- p->fail(std::current_exception()); //or state->failure();
+ p->tryFail(std::current_exception()); //or state->failure();
}
}
});
@@ -687,13 +635,13 @@ inline std::shared_ptr<celix::impl::SharedPromiseState<void>> celix::impl::Share
addOnResolve([p, recover = std::move(recover)](std::optional<std::exception_ptr> e) {
if (!e) {
- p->resolve();
+ p->tryResolve();
} else {
try {
recover();
- p->resolve();
+ p->tryResolve();
} catch (...) {
- p->fail(std::current_exception()); //or state->failure();
+ p->tryFail(std::current_exception()); //or state->failure();
}
}
});
@@ -710,15 +658,15 @@ std::shared_ptr<celix::impl::SharedPromiseState<T>> celix::impl::SharedPromiseSt
if (s->isSuccessfullyResolved()) {
try {
if (predicate(s->getValue())) {
- p->resolve(s->moveOrGetValue());
+ p->tryResolve(s->moveOrGetValue());
} else {
throw celix::PromiseInvocationException{"predicate does not accept value"};
}
} catch (...) {
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
};
addChain(std::move(chainFunction));
@@ -731,12 +679,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<T>> celix::impl::SharedPromiseSt
auto p = celix::impl::SharedPromiseState<T>::create(executor, scheduledExecutor, priority);
auto chainFunction = [s = self.lock(), p, fallbackTo = std::move(fallbackTo)] {
if (s->isSuccessfullyResolved()) {
- p->resolve(s->moveOrGetValue());
+ p->tryResolve(s->moveOrGetValue());
} else {
if (fallbackTo->isSuccessfullyResolved()) {
- p->resolve(fallbackTo->moveOrGetValue());
+ p->tryResolve(fallbackTo->moveOrGetValue());
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
}
};
@@ -749,13 +697,13 @@ inline std::shared_ptr<celix::impl::SharedPromiseState<void>> celix::impl::Share
auto chainFunction = [s = self.lock(), p, fallbackTo = std::move(fallbackTo)] {
if (s->isSuccessfullyResolved()) {
s->getValue();
- p->resolve();
+ p->tryResolve();
} else {
if (fallbackTo->isSuccessfullyResolved()) {
fallbackTo->getValue();
- p->resolve();
+ p->tryResolve();
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
}
};
@@ -808,12 +756,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<R>> celix::impl::SharedPromiseSt
auto chainFunction = [s = self.lock(), p, mapper = std::move(mapper)] {
try {
if (s->isSuccessfullyResolved()) {
- p->resolve(mapper(s->moveOrGetValue()));
+ p->tryResolve(mapper(s->moveOrGetValue()));
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
} catch (...) {
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
};
addChain(std::move(chainFunction));
@@ -830,12 +778,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<R>> celix::impl::SharedPromiseSt
try {
if (s->isSuccessfullyResolved()) {
s->getValue();
- p->resolve(mapper());
+ p->tryResolve(mapper());
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
} catch (...) {
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
};
addChain(std::move(chainFunction));
@@ -852,12 +800,12 @@ std::shared_ptr<celix::impl::SharedPromiseState<T>> celix::impl::SharedPromiseSt
if (s->isSuccessfullyResolved()) {
try {
consumer(s->getValue());
- p->resolve(s->moveOrGetValue());
+ p->tryResolve(s->moveOrGetValue());
} catch (...) {
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
};
addChain(std::move(chainFunction));
@@ -874,12 +822,12 @@ inline std::shared_ptr<celix::impl::SharedPromiseState<void>> celix::impl::Share
try {
s->getValue();
consumer();
- p->resolve();
+ p->tryResolve();
} catch (...) {
- p->fail(std::current_exception());
+ p->tryFail(std::current_exception());
}
} else {
- p->fail(s->getFailure());
+ p->tryFail(s->getFailure());
}
};
addChain(std::move(chainFunction));
diff --git a/libs/promises/gtest/src/PromisesTestSuite.cc b/libs/promises/gtest/src/PromisesTestSuite.cc
index 6469a24..7c18de7 100644
--- a/libs/promises/gtest/src/PromisesTestSuite.cc
+++ b/libs/promises/gtest/src/PromisesTestSuite.cc
@@ -575,7 +575,6 @@ TEST_F(PromiseTestSuite, outOfScopeUnresolvedPromises) {
TEST_F(PromiseTestSuite, chainPromises) {
auto success = [&](celix::Promise<long> p) -> celix::Promise<long> {
- //TODO Promises::resolved(p.getValue() + p.getValue())
auto result = factory->deferred<long>();
result.resolve(p.getValue() + p.getValue());
return result.getPromise();
@@ -668,6 +667,128 @@ TEST_F(PromiseTestSuite, getExecutorFromFactory) {
EXPECT_EQ(executor.get(), exec.get());
}
+TEST_F(PromiseTestSuite, testRobustFailAndResolve) {
+ std::atomic<int> failCount{};
+ std::atomic<int> successCount{};
+ auto failCb = [&failCount](const std::exception& /*e*/) {
+ failCount++;
+ };
+ auto successCb = [&successCount](int val) {
+ EXPECT_EQ(42, val);
+ successCount++;
+ };
+
+ auto def = factory->deferred<int>();
+ def.getPromise().onFailure(failCb);
+
+ //Rule a second fail should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.fail(std::logic_error{"error1"});
+ def.fail(std::logic_error{"error2"});
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+
+ def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb);
+ //Rule a second resolve should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.resolve(42);
+ def.resolve(43);
+ factory->wait();
+ EXPECT_EQ(successCount.load(), 1);
+
+
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ //Rule a resolve after fail should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.fail(std::logic_error("error3"));
+ def.resolve(43);
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+ EXPECT_EQ(successCount.load(), 0);
+
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ //Rule a fail after resolve should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.resolve(42);
+ def.fail(std::logic_error("error3"));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 0);
+ EXPECT_EQ(successCount.load(), 1);
+}
+
+TEST_F(PromiseTestSuite, testTryFailAndResolve) {
+ std::atomic<int> failCount{};
+ std::atomic<int> successCount{};
+ auto failCb = [&failCount](const std::exception& /*e*/) {
+ failCount++;
+ };
+ auto successCb = [&successCount](int val) {
+ EXPECT_EQ(42, val);
+ successCount++;
+ };
+ const int val = 42;
+
+ //first resolve with &&, then try rest
+ auto def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryResolve(42));
+ EXPECT_FALSE(def.tryResolve(43));
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 0);
+ EXPECT_EQ(successCount.load(), 1);
+
+ //first resolve with const int&, then try rest
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryResolve(val));
+ EXPECT_FALSE(def.tryResolve(43));
+ EXPECT_FALSE(def.tryResolve(val));
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 0);
+ EXPECT_EQ(successCount.load(), 1);
+
+ //first fail with exp ref, then try rest
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryResolve(43));
+ EXPECT_FALSE(def.tryResolve(val));
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+ EXPECT_EQ(successCount.load(), 0);
+
+ //first fail with exp ptr, then try rest
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<int>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ EXPECT_FALSE(def.tryResolve(43));
+ EXPECT_FALSE(def.tryResolve(val));
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+ EXPECT_EQ(successCount.load(), 0);
+}
+
#ifdef __clang__
#pragma clang diagnostic pop
#endif
diff --git a/libs/promises/gtest/src/VoidPromisesTestSuite.cc b/libs/promises/gtest/src/VoidPromisesTestSuite.cc
index 9accd54..6e25215 100644
--- a/libs/promises/gtest/src/VoidPromisesTestSuite.cc
+++ b/libs/promises/gtest/src/VoidPromisesTestSuite.cc
@@ -501,6 +501,108 @@ TEST_F(VoidPromiseTestSuite, deferredTaskCall) {
EXPECT_GT(durationInMs, std::chrono::milliseconds{10});
}
+TEST_F(VoidPromiseTestSuite, testRobustFailAndResolve) {
+ std::atomic<int> failCount{};
+ std::atomic<int> successCount{};
+ auto failCb = [&failCount](const std::exception& /*e*/) {
+ failCount++;
+ };
+ auto successCb = [&successCount]() {
+ successCount++;
+ };
+
+ auto def = factory->deferred<void>();
+ def.getPromise().onFailure(failCb);
+
+ //Rule a second fail should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.fail(std::logic_error{"error1"});
+ def.fail(std::logic_error{"error2"});
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+
+ def = factory->deferred<void>();
+ def.getPromise().onSuccess(successCb);
+ //Rule a second resolve should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.resolve();
+ def.resolve();
+ factory->wait();
+ EXPECT_EQ(successCount.load(), 1);
+
+
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<void>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ //Rule a resolve after fail should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.fail(std::logic_error("error3"));
+ def.resolve();
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+ EXPECT_EQ(successCount.load(), 0);
+
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<void>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ //Rule a fail after resolve should not lead to an exception, to ensure a more robust usage.
+ //But also should only lead to a single resolve chain.
+ def.resolve();
+ def.fail(std::logic_error("error3"));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 0);
+ EXPECT_EQ(successCount.load(), 1);
+}
+
+TEST_F(VoidPromiseTestSuite, testTryFailAndResolve) {
+ std::atomic<int> failCount{};
+ std::atomic<int> successCount{};
+ auto failCb = [&failCount](const std::exception& /*e*/) {
+ failCount++;
+ };
+ auto successCb = [&successCount]() {
+ successCount++;
+ };
+
+ //first resolve, then try rest
+ auto def = factory->deferred<void>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryResolve());
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 0);
+ EXPECT_EQ(successCount.load(), 1);
+
+ //first fail with exp ref, then try rest
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<void>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryResolve());
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+ EXPECT_EQ(successCount.load(), 0);
+
+ //first fail with exp ptr, then try rest
+ failCount = 0;
+ successCount = 0;
+ def = factory->deferred<void>();
+ def.getPromise().onSuccess(successCb).onFailure(failCb);
+ EXPECT_TRUE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ EXPECT_FALSE(def.tryResolve());
+ EXPECT_FALSE(def.tryFail(std::logic_error{"error"}));
+ EXPECT_FALSE(def.tryFail(std::make_exception_ptr(std::logic_error{"error"})));
+ factory->wait();
+ EXPECT_EQ(failCount.load(), 1);
+ EXPECT_EQ(successCount.load(), 0);
+}
+
#ifdef __clang__
#pragma clang diagnostic pop
#endif