This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/promise_timeout_memleak
in repository https://gitbox.apache.org/repos/asf/celix.git
The following commit(s) were added to
refs/heads/feature/promise_timeout_memleak by this push:
new 8b3149a Adds Promise<void> return to Deferred::resolveWith
8b3149a is described below
commit 8b3149ac6d1ce20bfb7d7dfed2d7f40a3559a8e2
Author: Pepijn Noltes <[email protected]>
AuthorDate: Mon Sep 6 20:25:39 2021 +0200
Adds Promise<void> return to Deferred::resolveWith
---
libs/promises/api/celix/Deferred.h | 60 +++++++++++-----------
.../api/celix/PromiseIllegalStateException.h | 33 ++++++++++++
libs/promises/api/celix/impl/SharedPromiseState.h | 26 ++++++----
libs/promises/gtest/src/PromisesTestSuite.cc | 34 +++++++++---
libs/promises/gtest/src/VoidPromisesTestSuite.cc | 47 +++++++++++++++--
5 files changed, 149 insertions(+), 51 deletions(-)
diff --git a/libs/promises/api/celix/Deferred.h
b/libs/promises/api/celix/Deferred.h
index c00a970..0f822b5 100644
--- a/libs/promises/api/celix/Deferred.h
+++ b/libs/promises/api/celix/Deferred.h
@@ -24,11 +24,12 @@
#include "celix/impl/SharedPromiseState.h"
#include "celix/Promise.h"
+#include "celix/PromiseIllegalStateException.h"
namespace celix {
/**
- * A Deferred Promise resolution.
+ * @brief A Deferred Promise resolution.
*
* <p>
* Instances of this class can be used to create a {@link celix::Promise}
that can be
@@ -104,7 +105,6 @@ namespace celix {
void resolve(T&& value);
void resolve(const T& value);
- //TODO update to resolveWith with a return celix::Promise<void> ??
/**
* Resolve the Promise associated with this Deferred with the
specified Promise.
* <p/>
@@ -126,7 +126,7 @@ namespace celix {
* associated Promise was already resolved when the specified Promise
was resolved.
*/
template<typename U>
- void resolveWith(celix::Promise<U> with);
+ celix::Promise<void> resolveWith(celix::Promise<U> with);
private:
std::shared_ptr<celix::impl::SharedPromiseState<T>> state;
@@ -193,13 +193,8 @@ namespace celix {
*/
void resolve();
- //TODO return Promise<void>
template<typename U>
- void resolveWith(celix::Promise<U> with);
-
- //TODO return Promise<void>
- void resolveWith(celix::Promise<void> with);
-
+ celix::Promise<void> resolveWith(celix::Promise<U> with);
private:
std::shared_ptr<celix::impl::SharedPromiseState<void>> state;
};
@@ -245,41 +240,44 @@ inline celix::Promise<void>
celix::Deferred<void>::getPromise() {
template<typename T>
template<typename U>
-void celix::Deferred<T>::resolveWith(celix::Promise<U> with) {
- with.onResolve([s = state, with] () mutable {
+celix::Promise<void> celix::Deferred<T>::resolveWith(celix::Promise<U> with) {
+ auto p =
celix::impl::SharedPromiseState<void>::create(state->getExecutor(),
state->getScheduledExecutor(), state->getPriority());
+ with.onResolve([s = state, with, p] () mutable {
+ bool resolved;
if (with.isSuccessfullyResolved()) {
- s->resolve(with.moveOrGetValue());
+ resolved = s->tryResolve(with.moveOrGetValue());
} else {
- s->fail(with.getFailure());
+ resolved = s->tryFail(with.getFailure());
+ }
+ if (resolved) {
+ p->tryResolve();
+ } else {
+ //s was already resolved
+
p->tryFail(std::make_exception_ptr(celix::PromiseIllegalStateException{}));
}
});
+ return celix::Promise<void>{p};
}
template<typename U>
-inline void celix::Deferred<void>::resolveWith(celix::Promise<U> with) {
- with.onResolve([s = state, with] {
+inline celix::Promise<void>
celix::Deferred<void>::resolveWith(celix::Promise<U> with) {
+ auto p =
celix::impl::SharedPromiseState<void>::create(state->getExecutor(),
state->getScheduledExecutor(), state->getPriority());
+ with.onResolve([s = state, with, p] {
+ bool resolved;
if (with.isSuccessfullyResolved()) {
with.getValue();
- s->resolve();
+ resolved = s->tryResolve();
} else {
- s->fail(with.getFailure());
- }
- });
-}
-
-inline void celix::Deferred<void>::resolveWith(celix::Promise<void> with) {
- with.onSuccess([s = state->getSelf()]() {
- auto l = s.lock();
- if (l) {
- l->resolve();
+ resolved = s->tryFail(with.getFailure());
}
- });
- with.onFailure([s = state->getSelf()](const std::exception& e) {
- auto l = s.lock();
- if (l) {
- l->fail(e);
+ if (resolved) {
+ p->tryResolve();
+ } else {
+ //s was already resolved
+
p->tryFail(std::make_exception_ptr(celix::PromiseIllegalStateException{}));
}
});
+ return celix::Promise<void>{p};
}
template<typename T>
diff --git a/libs/promises/api/celix/PromiseIllegalStateException.h
b/libs/promises/api/celix/PromiseIllegalStateException.h
new file mode 100644
index 0000000..17558e0
--- /dev/null
+++ b/libs/promises/api/celix/PromiseIllegalStateException.h
@@ -0,0 +1,33 @@
+/**
+ *Licensed to the Apache Software Foundation (ASF) under one
+ *or more contributor license agreements. See the NOTICE file
+ *distributed with this work for additional information
+ *regarding copyright ownership. The ASF licenses this file
+ *to you under the Apache License, Version 2.0 (the
+ *"License"); you may not use this file except in compliance
+ *with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *Unless required by applicable law or agreed to in writing,
+ *software distributed under the License is distributed on an
+ *"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ *specific language governing permissions and limitations
+ *under the License.
+ */
+
+#pragma once
+
+#include <exception>
+
+namespace celix {
+
+ class Ad : public std::exception {
+ public:
+ [[nodiscard]] const char* what() const noexcept override {
+ return "Illegal state";
+ }
+ };
+}
+
diff --git a/libs/promises/api/celix/impl/SharedPromiseState.h
b/libs/promises/api/celix/impl/SharedPromiseState.h
index b9edd3c..274d972 100644
--- a/libs/promises/api/celix/impl/SharedPromiseState.h
+++ b/libs/promises/api/celix/impl/SharedPromiseState.h
@@ -57,9 +57,9 @@ namespace celix::impl {
void fail(const std::exception &e);
- void tryResolve(T &&value);
+ bool tryResolve(T &&value);
- void tryFail(std::exception_ptr e);
+ bool tryFail(std::exception_ptr e);
// copy/move depending on situation
T& getValue() &;
@@ -148,7 +148,7 @@ namespace celix::impl {
public:
static std::shared_ptr<SharedPromiseState<void>>
create(std::shared_ptr<celix::IExecutor> _executor,
std::shared_ptr<celix::IScheduledExecutor> _scheduledExecutor, int priority);
- virtual ~SharedPromiseState() noexcept = default;
+ ~SharedPromiseState() noexcept = default;
void resolve();
@@ -156,9 +156,9 @@ namespace celix::impl {
void fail(const std::exception &e);
- void tryResolve();
+ bool tryResolve();
- void tryFail(std::exception_ptr e);
+ bool tryFail(std::exception_ptr e);
bool getValue() const;
std::exception_ptr getFailure() const;
@@ -342,39 +342,47 @@ inline void
celix::impl::SharedPromiseState<void>::fail(const std::exception& e)
}
template<typename T>
-void celix::impl::SharedPromiseState<T>::tryResolve(T&& value) {
+bool celix::impl::SharedPromiseState<T>::tryResolve(T&& value) {
std::unique_lock<std::mutex> lck{mutex};
if (!done) {
dataMoved = false;
data = std::forward<T>(value);
exp = nullptr;
complete(lck);
+ return true;
}
+ return false;
}
-inline void celix::impl::SharedPromiseState<void>::tryResolve() {
+inline bool celix::impl::SharedPromiseState<void>::tryResolve() {
std::unique_lock<std::mutex> lck{mutex};
if (!done) {
exp = nullptr;
complete(lck);
+ return true;
}
+ return false;
}
template<typename T>
-void celix::impl::SharedPromiseState<T>::tryFail(std::exception_ptr e) {
+bool celix::impl::SharedPromiseState<T>::tryFail(std::exception_ptr e) {
std::unique_lock<std::mutex> lck{mutex};
if (!done) {
exp = std::move(e);
complete(lck);
+ return true;
}
+ return false;
}
-inline void celix::impl::SharedPromiseState<void>::tryFail(std::exception_ptr
e) {
+inline bool celix::impl::SharedPromiseState<void>::tryFail(std::exception_ptr
e) {
std::unique_lock<std::mutex> lck{mutex};
if (!done) {
exp = std::move(e);
complete(lck);
+ return true;
}
+ return false;
}
template<typename T>
diff --git a/libs/promises/gtest/src/PromisesTestSuite.cc
b/libs/promises/gtest/src/PromisesTestSuite.cc
index ced4d90..799816b 100644
--- a/libs/promises/gtest/src/PromisesTestSuite.cc
+++ b/libs/promises/gtest/src/PromisesTestSuite.cc
@@ -203,7 +203,6 @@ TEST_F(PromiseTestSuite, onFailureHandling) {
TEST_F(PromiseTestSuite, resolveSuccessWith) {
auto deferred1 = factory->deferred<long>();
- auto deferred2 = factory->deferred<long>();
std::atomic<bool> called = false;
deferred1.getPromise()
.onSuccess([&](long value) {
@@ -211,11 +210,9 @@ TEST_F(PromiseTestSuite, resolveSuccessWith) {
called = true;
});
- //currently deferred1 will be resolved in thread, and onSuccess is trigger
on the promise of deferred2
- //now resolving deferred2 with the promise of deferred1
- deferred2.resolveWith(deferred1.getPromise());
- auto p = deferred2.getPromise();
- deferred1.resolve(42);
+ auto deferred2 = factory->deferred<long>();
+ deferred1.resolveWith(deferred2.getPromise());
+ deferred2.resolve(42);
factory->wait();
EXPECT_EQ(true, called);
@@ -248,6 +245,31 @@ TEST_F(PromiseTestSuite, resolveFailureWith) {
EXPECT_EQ(true, failureCalled);
}
+TEST_F(PromiseTestSuite, returnPromiseOnSuccessfulResolveWith) {
+ auto deferred1 = factory->deferred<long>();
+ auto deferred2 = factory->deferred<long>();
+ celix::Promise<void> promise =
deferred1.resolveWith(deferred2.getPromise());
+ EXPECT_FALSE(promise.isDone());
+ deferred2.resolve(42);
+ factory->wait();
+ EXPECT_TRUE(promise.isDone());
+ EXPECT_TRUE(promise.isSuccessfullyResolved());
+}
+
+TEST_F(PromiseTestSuite, returnPromiseOnInvalidResolveWith) {
+ auto deferred1 = factory->deferred<long>();
+ auto deferred2 = factory->deferred<long>();
+ celix::Promise<void> promise =
deferred1.resolveWith(deferred2.getPromise());
+ EXPECT_FALSE(promise.isDone());
+ deferred1.resolve(42); //Rule is deferred1 is resolved, before the
resolveWith the returned promise should fail
+ deferred2.resolve(42);
+ factory->wait();
+ EXPECT_TRUE(promise.isDone());
+ EXPECT_FALSE(promise.isSuccessfullyResolved());
+ EXPECT_THROW(std::rethrow_exception(promise.getFailure()),
celix::PromiseIllegalStateException);
+}
+
+
//gh-333 fix timeout test on macos
#ifndef __APPLE__
TEST_F(PromiseTestSuite, resolveWithTimeout) {
diff --git a/libs/promises/gtest/src/VoidPromisesTestSuite.cc
b/libs/promises/gtest/src/VoidPromisesTestSuite.cc
index be016b4..f2efdb5 100644
--- a/libs/promises/gtest/src/VoidPromisesTestSuite.cc
+++ b/libs/promises/gtest/src/VoidPromisesTestSuite.cc
@@ -128,22 +128,35 @@ TEST_F(VoidPromiseTestSuite, onFailureHandling) {
TEST_F(VoidPromiseTestSuite, resolveSuccessWith) {
auto deferred1 = factory->deferred<void>();
- auto deferred2 = factory->deferred<long>();
-
bool called = false;
deferred1.getPromise()
.onSuccess([&called]() {
called = true;
});
- //currently deferred1 will be resolved in thread, and onSuccess is trigger
on the promise of deferred2
- //now resolving deferred2 with the promise of deferred1
+ auto deferred2 = factory->deferred<int>();
deferred1.resolveWith(deferred2.getPromise());
- deferred2.resolve(1);
+ deferred2.resolve(42);
+
+ factory->wait();
+ EXPECT_EQ(true, called);
+
+ auto deferred3 = factory->deferred<void>();
+ called = false;
+ deferred3.getPromise()
+ .onSuccess([&called]() {
+ called = true;
+ });
+
+ auto deferred4 = factory->deferred<void>();
+ deferred3.resolveWith(deferred4.getPromise());
+ deferred4.resolve();
+
factory->wait();
EXPECT_EQ(true, called);
}
+
TEST_F(VoidPromiseTestSuite, resolveFailureWith) {
auto deferred1 = factory->deferred<void>();
auto deferred2 = factory->deferred<void>();
@@ -173,6 +186,30 @@ TEST_F(VoidPromiseTestSuite, resolveFailureWith) {
EXPECT_EQ(true, failureCalled);
}
+TEST_F(VoidPromiseTestSuite, returnPromiseOnSuccessfulResolveWith) {
+ auto deferred1 = factory->deferred<void>();
+ auto deferred2 = factory->deferred<void>();
+ celix::Promise<void> promise =
deferred1.resolveWith(deferred2.getPromise());
+ EXPECT_FALSE(promise.isDone());
+ deferred2.resolve();
+ factory->wait();
+ EXPECT_TRUE(promise.isDone());
+ EXPECT_TRUE(promise.isSuccessfullyResolved());
+}
+
+TEST_F(VoidPromiseTestSuite, returnPromiseOnInvalidResolveWith) {
+ auto deferred1 = factory->deferred<void>();
+ auto deferred2 = factory->deferred<long>();
+ celix::Promise<void> promise =
deferred1.resolveWith(deferred2.getPromise());
+ EXPECT_FALSE(promise.isDone());
+ deferred1.resolve(); //Rule is deferred1 is resolved, before the
resolveWith the returned promise should fail
+ deferred2.resolve(42);
+ factory->wait();
+ EXPECT_TRUE(promise.isDone());
+ EXPECT_FALSE(promise.isSuccessfullyResolved());
+ EXPECT_THROW(std::rethrow_exception(promise.getFailure()),
celix::PromiseIllegalStateException);
+}
+
//gh-333 fix timeout test on macos
#ifndef __APPLE__
TEST_F(VoidPromiseTestSuite, resolveWithTimeout) {
