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:42 UTC
[celix] branch feature/robust_promise_fail_and_resolve created (now
4329677)
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a change to branch feature/robust_promise_fail_and_resolve
in repository https://gitbox.apache.org/repos/asf/celix.git.
at 4329677 Updates Promise `resolve` and `fail` so make them robust.
This branch includes the following new commits:
new 4329677 Updates Promise `resolve` and `fail` so make them robust.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[celix] 01/01: Updates Promise `resolve` and `fail` so make them
robust.
Posted by pn...@apache.org.
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