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 2023/06/28 15:23:44 UTC
[celix] 28/42: Fix some issues with the introduction of scheduled event
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/scheduled_event_on_event_thread
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 64cbc81934c675225019d137616da80fbd3f117f
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Sun Jun 18 18:55:31 2023 +0200
Fix some issues with the introduction of scheduled event
---
.../log_admin/gtest/src/LogAdminTestSuite.cc | 4 --
documents/scheduled_events .md | 4 +-
.../src/ScheduleEventsBundleActivator.cc | 4 +-
.../framework/gtest/src/CelixFrameworkTestSuite.cc | 6 +-
.../framework/gtest/src/ScheduledEventTestSuite.cc | 22 +++----
libs/framework/include/celix_framework.h | 5 +-
libs/framework/src/celix_scheduled_event.c | 31 +++-------
libs/framework/src/celix_scheduled_event.h | 21 +++----
libs/framework/src/framework.c | 69 ++++++++++++++--------
libs/utils/gtest/src/ThreadsTestSuite.cc | 44 ++------------
libs/utils/include/celix_threads.h | 60 ++++++++++---------
libs/utils/src/celix_threads.c | 35 +++--------
libs/utils/src/utils.c | 1 -
13 files changed, 127 insertions(+), 179 deletions(-)
diff --git a/bundles/logging/log_admin/gtest/src/LogAdminTestSuite.cc b/bundles/logging/log_admin/gtest/src/LogAdminTestSuite.cc
index 95b75a81..9f0d403e 100644
--- a/bundles/logging/log_admin/gtest/src/LogAdminTestSuite.cc
+++ b/bundles/logging/log_admin/gtest/src/LogAdminTestSuite.cc
@@ -342,10 +342,6 @@ TEST_F(LogBundleTestSuite, LogServiceAndSink) {
}
celix_framework_waitForEmptyEventQueue(fw.get());
- //TODO fixme, apparently this is needed, because the celix_framework_waitForEmptyEventQueue for does not work
- //this is probably due to the moved broadcast in the framework event handle, check this
- std::this_thread::sleep_for(std::chrono::milliseconds(10));
-
ASSERT_TRUE(logSvc.load() != nullptr);
auto initial = count.load();
celix_log_service_t *ls = logSvc.load();
diff --git a/documents/scheduled_events .md b/documents/scheduled_events .md
index d01dfeba..8d8fac32 100644
--- a/documents/scheduled_events .md
+++ b/documents/scheduled_events .md
@@ -103,7 +103,7 @@ public:
explicit ScheduleEventsBundleActivator(const std::shared_ptr<celix::BundleContext>& ctx) {
//schedule recurring event
event = ctx->scheduledEvent()
- .withInitialDelay(std::chrono::milliseconds {10})
+ .withInitialDelay(std::chrono::milliseconds{10})
.withInterval(std::chrono::seconds{1})
.withCallback([ctx] {
ctx->logInfo("Recurring scheduled event fired");
@@ -112,7 +112,7 @@ public:
//schedule one time event
ctx->scheduledEvent()
- .withInitialDelay(std::chrono::milliseconds {10})
+ .withInitialDelay(std::chrono::milliseconds{10})
.withCallback([ctx] {
ctx->logInfo("One shot scheduled event fired");
})
diff --git a/examples/celix-examples/readme_cxx_examples/src/ScheduleEventsBundleActivator.cc b/examples/celix-examples/readme_cxx_examples/src/ScheduleEventsBundleActivator.cc
index 95bf0d12..fa4d7415 100644
--- a/examples/celix-examples/readme_cxx_examples/src/ScheduleEventsBundleActivator.cc
+++ b/examples/celix-examples/readme_cxx_examples/src/ScheduleEventsBundleActivator.cc
@@ -25,7 +25,7 @@ public:
explicit ScheduleEventsBundleActivator(const std::shared_ptr<celix::BundleContext>& ctx) {
//schedule recurring event
event = ctx->scheduledEvent()
- .withInitialDelay(std::chrono::milliseconds {10})
+ .withInitialDelay(std::chrono::milliseconds{10})
.withInterval(std::chrono::seconds{1})
.withCallback([ctx] {
ctx->logInfo("Recurring scheduled event fired");
@@ -34,7 +34,7 @@ public:
//schedule one time event
ctx->scheduledEvent()
- .withInitialDelay(std::chrono::milliseconds {10})
+ .withInitialDelay(std::chrono::milliseconds{10})
.withCallback([ctx] {
ctx->logInfo("One shot scheduled event fired");
})
diff --git a/libs/framework/gtest/src/CelixFrameworkTestSuite.cc b/libs/framework/gtest/src/CelixFrameworkTestSuite.cc
index 7085f7b4..db9a168a 100644
--- a/libs/framework/gtest/src/CelixFrameworkTestSuite.cc
+++ b/libs/framework/gtest/src/CelixFrameworkTestSuite.cc
@@ -90,11 +90,11 @@ TEST_F(CelixFrameworkTestSuite, TimedWaitEventQueueTest) {
celix_framework_fireGenericEvent(framework.get(), -1L, -1L, "test", nullptr, callback, nullptr, nullptr);
//Then a wait for empty event queue for max 5ms will return a timeout
- celix_status_t status = celix_framework_timedWaitForEmptyEventQueue(framework.get(), 0.005);
- EXPECT_EQ(ETIMEDOUT, status);
+ celix_status_t status = celix_framework_waitForEmptyEventQueueFor(framework.get(), 0.005);
+ EXPECT_EQ(ETIMEDOUT, status) << "Expected timeout, but got " << celix_strerror(status);
//And a wait for empty event queue for max 30ms will return success
- status = celix_framework_timedWaitForEmptyEventQueue(framework.get(), 0.03);
+ status = celix_framework_waitForEmptyEventQueueFor(framework.get(), 0.03);
EXPECT_EQ(CELIX_SUCCESS, status);
}
diff --git a/libs/framework/gtest/src/ScheduledEventTestSuite.cc b/libs/framework/gtest/src/ScheduledEventTestSuite.cc
index 8fe61903..fd7eac03 100644
--- a/libs/framework/gtest/src/ScheduledEventTestSuite.cc
+++ b/libs/framework/gtest/src/ScheduledEventTestSuite.cc
@@ -28,7 +28,7 @@ class ScheduledEventTestSuite : public ::testing::Test {
ScheduledEventTestSuite() { fw = celix::createFramework({{"CELIX_LOGGING_DEFAULT_ACTIVE_LOG_LEVEL", "trace"}}); }
std::shared_ptr<celix::Framework> fw{};
- static constexpr int ALLOWED_TIMED_WAIT_ERROR_MS = 40;
+ const int ALLOWED_TIMED_WAIT_ERROR_MS = 40;
/**
* Wait for the given predicate to become true or the given time has elapsed.
@@ -79,12 +79,15 @@ TEST_F(ScheduledEventTestSuite, OnceShotEventTest) {
long eventId = celix_bundleContext_scheduleEvent(ctx->getCBundleContext(), &opts);
EXPECT_GE(eventId, 0);
- // Then the event is called once
- std::this_thread::sleep_for(std::chrono::milliseconds{10});
+ // Then the event is called once within the error margin
+ waitFor([&]() { return info.count.load() == 1; }, std::chrono::milliseconds{ALLOWED_TIMED_WAIT_ERROR_MS});
EXPECT_EQ(1, info.count.load());
- // And the event remove callback is called
+ // And the event remove callback is called within the error margin
+ waitFor([&]() { return info.removed.load(); }, std::chrono::milliseconds{ALLOWED_TIMED_WAIT_ERROR_MS});
EXPECT_TRUE(info.removed.load());
+
+ celix_bundleContext_removeScheduledEvent(ctx->getCBundleContext(), eventId);
}
TEST_F(ScheduledEventTestSuite, ScheduledEventTest) {
@@ -120,10 +123,9 @@ TEST_F(ScheduledEventTestSuite, ScheduledEventTest) {
long eventId = celix_bundleContext_scheduleEvent(ctx->getCBundleContext(), &opts);
EXPECT_GE(eventId, 0);
- // And wait more than 10 ms + 2x 20ms + 10ms error margin
- std::this_thread::sleep_for(std::chrono::milliseconds{60});
-
- // Then the event is called at least 3 times
+ // Then count becomes 3 or more within the initial delay + 2 x internal and an allowed error margin
+ int allowedTimeInMs = 10 + (2 * 20) + ALLOWED_TIMED_WAIT_ERROR_MS;
+ waitFor([&]() { return info.count.load() >= 3; }, std::chrono::milliseconds{allowedTimeInMs});
EXPECT_GE(info.count.load(), 3);
// And the event remove callback is not called
@@ -452,7 +454,7 @@ TEST_F(ScheduledEventTestSuite, CxxOneShotScheduledEventTest) {
EXPECT_FALSE(removed.load());
// And count will be increased within the initial delay (including some error margin)
- waitFor([&]{return count.load() == 1;}, std::chrono::milliseconds{60});
+ waitFor([&]{return count.load() == 1;}, std::chrono::milliseconds{50 + ALLOWED_TIMED_WAIT_ERROR_MS});
EXPECT_EQ(1, count.load());
// And the remove callback is called shortly after the initial delay
@@ -498,7 +500,7 @@ TEST_F(ScheduledEventTestSuite, CxxOneShotScheduledEventRAIITest) {
EXPECT_FALSE(removed.load());
// And count will be increased within the initial delay (including some error margin)
- waitFor([&]{return count.load() == 1;}, std::chrono::milliseconds{60});
+ waitFor([&]{return count.load() == 1;}, std::chrono::milliseconds{50 + ALLOWED_TIMED_WAIT_ERROR_MS});
EXPECT_EQ(1, count.load());
// And the remove callback is called shortly after the initial delay
diff --git a/libs/framework/include/celix_framework.h b/libs/framework/include/celix_framework.h
index d1d47cf5..749752f7 100644
--- a/libs/framework/include/celix_framework.h
+++ b/libs/framework/include/celix_framework.h
@@ -350,11 +350,12 @@ CELIX_FRAMEWORK_EXPORT void celix_framework_waitForEmptyEventQueue(celix_framewo
* @param[in] timeoutInSeconds The period in seconds to wait for the event queue to be empty. 0 means wait forever.
* @return CELIX_SUCCESS if the event queue is empty or ETIMEDOUT if the timeoutInSeconds is reached.
*/
-CELIX_FRAMEWORK_EXPORT celix_status_t celix_framework_timedWaitForEmptyEventQueue(celix_framework_t *fw, double timeoutInSeconds);
+CELIX_FRAMEWORK_EXPORT celix_status_t celix_framework_waitForEmptyEventQueueFor(celix_framework_t *fw, double timeoutInSeconds);
/**
* @brief wait until all events from the event queue for the bundle identified by the bndId are processed.
- *
+ *
+ * If bndId < 0, wait until all events from the event queue are processed.
* Note scheduled events are not part of the event queue.
*
*/
diff --git a/libs/framework/src/celix_scheduled_event.c b/libs/framework/src/celix_scheduled_event.c
index ab4a065d..f02c09d3 100644
--- a/libs/framework/src/celix_scheduled_event.c
+++ b/libs/framework/src/celix_scheduled_event.c
@@ -182,11 +182,11 @@ bool celix_scheduledEvent_deadlineReached(celix_scheduled_event_t* event,
deadlineReached = true;
}
- if (deadlineReached) {
+ if (deadlineReached && nextProcessTimeInSeconds) {
*nextProcessTimeInSeconds =
event->intervalInSeconds == 0 /*one shot*/ ? CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS
: event->intervalInSeconds;
- } else {
+ } else if (nextProcessTimeInSeconds) {
*nextProcessTimeInSeconds = event->callCount == 0 ? event->initialDelayInSeconds : event->intervalInSeconds;
}
celixThreadMutex_unlock(&event->mutex);
@@ -211,11 +211,9 @@ void celix_scheduledEvent_process(celix_scheduled_event_t* event, const struct t
celixThreadMutex_unlock(&event->mutex);
}
-bool celix_scheduledEvent_isSingleShot(celix_scheduled_event_t* event) {
+bool celix_scheduledEvent_isSingleShot(const celix_scheduled_event_t* event) {
bool isDone = false;
- celixThreadMutex_lock(&event->mutex);
isDone = event->intervalInSeconds == 0;
- celixThreadMutex_unlock(&event->mutex);
return isDone;
}
@@ -235,24 +233,6 @@ size_t celix_scheduledEvent_markForWakeup(celix_scheduled_event_t* event) {
return currentCallCount + 1;
}
-celix_status_t celix_scheduledEvent_waitForAtLeastCallCount(celix_scheduled_event_t* event,
- size_t targetCallCount,
- double waitTimeInSeconds) {
- celix_status_t status = CELIX_SUCCESS;
- if (waitTimeInSeconds > 0) {
- struct timespec absTime= celixThreadCondition_getDelayedTime(waitTimeInSeconds);
- celixThreadMutex_lock(&event->mutex);
- while (event->callCount < targetCallCount) {
- status = celixThreadCondition_waitUntil(&event->cond, &event->mutex, &absTime);
- if (status == ETIMEDOUT) {
- break;
- }
- }
- celixThreadMutex_unlock(&event->mutex);
- }
- return status;
-}
-
void celix_scheduledEvent_waitForRemoved(celix_scheduled_event_t* event) {
struct timespec absLogTimeout = celixThreadCondition_getDelayedTime(CELIX_SCHEDULED_EVENT_ERROR_LOG_TIMEOUT_IN_SECONDS);
celixThreadMutex_lock(&event->mutex);
@@ -308,3 +288,8 @@ bool celix_scheduledEvent_isMarkedForRemoval(celix_scheduled_event_t* event) {
celixThreadMutex_unlock(&event->mutex);
return isMarkedForRemoval;
}
+
+bool celix_scheduledEvent_requiresProcessing(celix_scheduled_event_t* event, const struct timespec* currentTime) {
+ return celix_scheduledEvent_deadlineReached(event, currentTime, NULL) ||
+ celix_scheduledEvent_isMarkedForRemoval(event);
+}
\ No newline at end of file
diff --git a/libs/framework/src/celix_scheduled_event.h b/libs/framework/src/celix_scheduled_event.h
index 6446fcbe..27ae6cc2 100644
--- a/libs/framework/src/celix_scheduled_event.h
+++ b/libs/framework/src/celix_scheduled_event.h
@@ -119,7 +119,7 @@ void celix_scheduledEvent_waitForRemoved(celix_scheduled_event_t* event);
/**
* @brief Returns true if the event is a one-shot event.
*/
-bool celix_scheduledEvent_isSingleShot(celix_scheduled_event_t* event);
+bool celix_scheduledEvent_isSingleShot(const celix_scheduled_event_t* event);
/**
* @brief Mark the event for removal. The event will be removed on the event thread, after the next processing.
@@ -140,18 +140,6 @@ bool celix_scheduledEvent_isMarkedForRemoval(celix_scheduled_event_t* event);
*/
size_t celix_scheduledEvent_markForWakeup(celix_scheduled_event_t* event);
-/**
- * @brief Wait for a scheduled event to reach at least the provided call count.
- * Will directly (non blocking) return if the call count is already reached or waitTimeInSeconds is <= 0
- * @param[in] event The event to wait for.
- * @param[in] callCount The call count to wait for.
- * @param[in] timeout The max time to wait in seconds.
- * @return CELIX_SUCCESS if the scheduled event reached the call count, ETIMEDOUT if the scheduled event
- */
-celix_status_t celix_scheduledEvent_waitForAtLeastCallCount(celix_scheduled_event_t* event,
- size_t targetCallCount,
- double waitTimeInSeconds);
-
/**
* @brief Wait for a scheduled event to be done with the next scheduled processing.
* @param[in] event The event to wait for.
@@ -161,6 +149,13 @@ celix_status_t celix_scheduledEvent_waitForAtLeastCallCount(celix_scheduled_even
*/
celix_status_t celix_scheduledEvent_wait(celix_scheduled_event_t* event, double timeoutInSeconds);
+
+/**
+ * @brief Returns true if the event is marked for wakeup, the initial delay or interval deadline is reached or the
+ * event is marked for removal for the given time.
+ */
+bool celix_scheduledEvent_requiresProcessing(celix_scheduled_event_t* event, const struct timespec* currentTime);
+
#ifdef __cplusplus
};
#endif
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index 62dfbf95..523e2a3b 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -57,6 +57,7 @@ struct celix_bundle_activator {
celix_bundle_activator_destroy_fp destroy;
};
+static int celix_framework_eventQueueSize(celix_framework_t* fw);
static celix_status_t celix_framework_stopBundleEntryInternal(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry);
static inline celix_framework_bundle_entry_t* fw_bundleEntry_create(celix_bundle_t *bnd) {
@@ -1500,7 +1501,7 @@ static double celix_framework_processScheduledEvents(celix_framework_t* fw) {
celix_scheduledEvent_setRemoved(removeEvent);
celix_scheduledEvent_release(removeEvent);
}
- } while (callEvent != NULL && removeEvent != NULL);
+ } while (callEvent || removeEvent);
return nextClosestScheduledEvent;
}
@@ -1538,18 +1539,36 @@ void celix_framework_cleanupScheduledEvents(celix_framework_t* fw, long bndId) {
} while (removeEvent != NULL);
}
+static int celix_framework_eventQueueSize(celix_framework_t* fw) {
+ //precondition fw->dispatcher.mutex lockedx);
+ return fw->dispatcher.eventQueueSize + celix_arrayList_size(fw->dispatcher.dynamicEventQueue);
+}
+
+static bool requiresScheduledEventsProcessing(celix_framework_t* framework) {
+ // precondition framework->dispatcher.mutex locked
+ struct timespec currentTime = celixThreadCondition_getTime();
+ bool eventProcessingRequired = false;
+ CELIX_LONG_HASH_MAP_ITERATE(framework->dispatcher.scheduledEvents, mapEntry) {
+ celix_scheduled_event_t* visit = mapEntry.value.ptrValue;
+ if (celix_scheduledEvent_requiresProcessing(visit, ¤tTime)) {
+ eventProcessingRequired = true;
+ break;
+ }
+ }
+ return eventProcessingRequired;
+}
+
static void celix_framework_waitForNextEvent(celix_framework_t* fw, double nextScheduledEvent) {
- long seconds = CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS;
- long nanoseconds = 0;
- if (nextScheduledEvent > 0) {
- seconds = (long) nextScheduledEvent;
- nanoseconds = ((long)(nextScheduledEvent * 1000000000L)) - seconds;
+ if (nextScheduledEvent < 0 || nextScheduledEvent > CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS) {
+ nextScheduledEvent = CELIX_FRAMEWORK_DEFAULT_MAX_TIMEDWAIT_EVENT_HANDLER_IN_SECONDS;
}
+ struct timespec absTimeout = celixThreadCondition_getDelayedTime(nextScheduledEvent);
celixThreadMutex_lock(&fw->dispatcher.mutex);
- int size = fw->dispatcher.eventQueueSize + celix_arrayList_size(fw->dispatcher.dynamicEventQueue);
- if (size == 0 && fw->dispatcher.active) {
- celixThreadCondition_timedwaitRelative(&fw->dispatcher.cond, &fw->dispatcher.mutex, seconds, nanoseconds);
+ if (celix_framework_eventQueueSize(fw) == 0 && !requiresScheduledEventsProcessing(fw) && fw->dispatcher.active) {
+ celixThreadCondition_waitUntil(&fw->dispatcher.cond, &fw->dispatcher.mutex, &absTimeout);
+ // note failing through to fw_eventDispatcher even if timeout is not reached, the fw_eventDispatcher
+ // will call this again after processing the events and scheduled events.
}
celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
@@ -1571,12 +1590,15 @@ static void *fw_eventDispatcher(void *fw) {
celixThreadMutex_unlock(&framework->dispatcher.mutex);
}
- //not active any more, last run for possible request leftovers
+ //not active anymore, extra runs for possible request leftovers
celixThreadMutex_lock(&framework->dispatcher.mutex);
- bool needLastRun = framework->dispatcher.eventQueueSize > 0 || celix_arrayList_size(framework->dispatcher.dynamicEventQueue) > 0;
+ bool needExtraRun = celix_framework_eventQueueSize(fw) > 0;
celixThreadMutex_unlock(&framework->dispatcher.mutex);
- if (needLastRun) {
+ while (needExtraRun) {
fw_handleEvents(framework);
+ celixThreadMutex_lock(&framework->dispatcher.mutex);
+ needExtraRun = celix_framework_eventQueueSize(fw) > 0;
+ celixThreadMutex_unlock(&framework->dispatcher.mutex);
}
celixThread_exit(NULL);
@@ -2513,30 +2535,29 @@ celix_array_list_t* celix_framework_listInstalledBundles(celix_framework_t* fram
return celix_framework_listBundlesInternal(framework, false);
}
-celix_status_t celix_framework_timedWaitForEmptyEventQueue(celix_framework_t *fw, double periodInSeconds) {
+celix_status_t celix_framework_waitForEmptyEventQueueFor(celix_framework_t *fw, double periodInSeconds) {
assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
-
celix_status_t status = CELIX_SUCCESS;
- long seconds = (long) periodInSeconds;
- long nanoseconds = (long) ((periodInSeconds - (double)seconds) * 1000000000L);
+ struct timespec absTimeout = celixThreadCondition_getDelayedTime(periodInSeconds);
celixThreadMutex_lock(&fw->dispatcher.mutex);
- while (fw->dispatcher.eventQueueSize > 0 || celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
- if (periodInSeconds > 0) {
- status = celixThreadCondition_timedwaitRelative(&fw->dispatcher.cond, &fw->dispatcher.mutex, seconds, nanoseconds);
+ while (celix_framework_eventQueueSize(fw) > 0) {
+ if (periodInSeconds == 0) {
+ celixThreadCondition_wait(&fw->dispatcher.cond, &fw->dispatcher.mutex);
+ } else {
+ status = celixThreadCondition_waitUntil(&fw->dispatcher.cond, &fw->dispatcher.mutex, &absTimeout);
if (status == ETIMEDOUT) {
break;
}
- } else {
- celixThreadCondition_wait(&fw->dispatcher.cond, &fw->dispatcher.mutex);
}
+
}
celixThreadMutex_unlock(&fw->dispatcher.mutex);
return status;
}
void celix_framework_waitForEmptyEventQueue(celix_framework_t *fw) {
- celix_framework_waitUntilNoEventsForBnd(fw, 0);
+ celix_framework_waitUntilNoEventsForBnd(fw, -1);
}
void celix_framework_waitUntilNoEventsForBnd(celix_framework_t* fw, long bndId) {
@@ -2549,14 +2570,14 @@ void celix_framework_waitUntilNoEventsForBnd(celix_framework_t* fw, long bndId)
for (int i = 0; i < fw->dispatcher.eventQueueSize; ++i) {
int index = (fw->dispatcher.eventQueueFirstEntry + i) % fw->dispatcher.eventQueueCap;
celix_framework_event_t* e = &fw->dispatcher.eventQueue[index];
- if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ if (e->bndEntry != NULL && (bndId < 0 || e->bndEntry->bndId == bndId)) {
eventInProgress = true;
break;
}
}
for (int i = 0; !eventInProgress && i < celix_arrayList_size(fw->dispatcher.dynamicEventQueue); ++i) {
celix_framework_event_t* e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, i);
- if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ if (e->bndEntry != NULL && (bndId < 0 || e->bndEntry->bndId == bndId)) {
eventInProgress = true;
break;
}
diff --git a/libs/utils/gtest/src/ThreadsTestSuite.cc b/libs/utils/gtest/src/ThreadsTestSuite.cc
index 14f7ce9a..2cbef44d 100644
--- a/libs/utils/gtest/src/ThreadsTestSuite.cc
+++ b/libs/utils/gtest/src/ThreadsTestSuite.cc
@@ -106,7 +106,7 @@ TEST_F(ThreadsTestSuite, InitializedTest) {
celixThread_join(thread, nullptr);
}
-TEST_F(ThreadsTestSuite, CnceTest) {
+TEST_F(ThreadsTestSuite, OnceTest) {
int *status;
celix_thread_t thread;
celix_thread_t thread2;
@@ -283,23 +283,13 @@ TEST_F(ThreadsTestSuite, CondTimedWaitTest) {
celix_thread_mutex_t mutex;
celix_thread_cond_t cond;
- auto status = celixThreadMutex_create(&mutex, NULL);
+ auto status = celixThreadMutex_create(&mutex, nullptr);
ASSERT_EQ(status, CELIX_SUCCESS);
- status = celixThreadCondition_init(&cond, NULL);
+ status = celixThreadCondition_init(&cond, nullptr);
ASSERT_EQ(status, CELIX_SUCCESS);
- //Test with NULL abstime
- status = celixThreadCondition_waitUntil(&cond, &mutex, NULL);
- ASSERT_EQ(status, CELIX_ILLEGAL_ARGUMENT);
-
- //: Test with negative tv_sec
- struct timespec abstime = {-1, 0};
- status = celixThreadCondition_waitUntil(&cond, &mutex, &abstime);
- ASSERT_EQ(status, CELIX_ILLEGAL_ARGUMENT);
-
- //Test with negative tv_nsec
- abstime = {0, -1};
- status = celixThreadCondition_waitUntil(&cond, &mutex, &abstime);
+ //Test with nullptr abstime
+ status = celixThreadCondition_waitUntil(&cond, &mutex, nullptr);
ASSERT_EQ(status, CELIX_ILLEGAL_ARGUMENT);
//Test with valid abstime
@@ -313,30 +303,6 @@ TEST_F(ThreadsTestSuite, CondTimedWaitTest) {
EXPECT_NEAR(celix_difftime(&end, &start), 0.001, 0.02);
}
-TEST_F(ThreadsTestSuite, CondWaitForTest) {
- celix_thread_mutex_t mutex;
- celix_thread_cond_t cond;
-
- auto status = celixThreadMutex_create(&mutex, NULL);
- ASSERT_EQ(status, CELIX_SUCCESS);
- status = celixThreadCondition_init(&cond, NULL);
- ASSERT_EQ(status, CELIX_SUCCESS);
-
- //Test with negative timeout
- status = celixThreadCondition_waitFor(&cond, &mutex, -1);
- ASSERT_EQ(status, CELIX_ILLEGAL_ARGUMENT);
-
- //Test with valid delay
- auto start = celix_gettime(CLOCK_MONOTONIC);
- pthread_mutex_lock(&mutex);
- status = celixThreadCondition_waitFor(&cond, &mutex, 0.001);
- ASSERT_EQ(status, ETIMEDOUT);
- pthread_mutex_unlock(&mutex);
- auto end = celix_gettime(CLOCK_MONOTONIC);
- EXPECT_NEAR(celix_difftime(&end, &start), 0.001, 0.01);
-}
-
-
//----------------------CELIX READ-WRITE LOCK TESTS----------------------
TEST_F(ThreadsTestSuite, CreateRwLockTest) {
diff --git a/libs/utils/include/celix_threads.h b/libs/utils/include/celix_threads.h
index 5eb1b584..57cd5c53 100644
--- a/libs/utils/include/celix_threads.h
+++ b/libs/utils/include/celix_threads.h
@@ -122,56 +122,58 @@ CELIX_UTILS_EXPORT celix_status_t celixThreadRwlockAttr_destroy(celix_thread_rwl
typedef pthread_cond_t celix_thread_cond_t;
typedef pthread_condattr_t celix_thread_condattr_t;
+/**
+ * @brief Initialize the given condition variable.
+ *
+ * For Linux the condition clock is set to CLOCK_MONOTONIC whether or not the attr is NULL.
+ *
+ * @param[in] condition The condition variable to initialize.
+ * @param[in] attr The condition variable attributes to use. Can be NULL for default attributes.
+ * @return CELIX_SUCCESS if the condition variable is initialized successfully.
+ */
CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_init(celix_thread_cond_t *condition, celix_thread_condattr_t *attr);
CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_destroy(celix_thread_cond_t *condition);
CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_wait(celix_thread_cond_t *cond, celix_thread_mutex_t *mutex);
-CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_timedwaitRelative(celix_thread_cond_t *cond, celix_thread_mutex_t *mutex, long seconds, long nanoseconds);
-
/**
- * @brief Wait for the condition to be signaled or until the given delayInSeconds is reached.
- *
- * @section Errors
- * - CELIX_SUCCESS if the condition is signaled before the delayInSeconds is reached.
- * - CELIX_ILLEGAL_ARGUMENT if the delayInSeconds is negative.
- * - ENOTRECOVERABLE if the state protected by the mutex is not recoverable.
- * - ETIMEDOUT If the delayInSeconds has passed.
- *
- *
- * @param[in] cond The condition to wait for.
- * @param[in] mutex The (locked) mutex to use.
- * @param[in] delayInSeconds The delay in seconds to wait for the condition to be signaled.
- * @return CELIX_SUCCESS if the condition is signaled before the delayInSeconds is reached.
+ * @brief Wait until the given time.
+ * @deprecated use celixThreadCondition_waitUntil.
*/
-CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_waitFor(celix_thread_cond_t* cond,
- celix_thread_mutex_t* mutex,
- double delayInSeconds);
+CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_timedwaitRelative(celix_thread_cond_t *cond, celix_thread_mutex_t *mutex, long seconds, long nanoseconds) CELIX_UTILS_DEPRECATED;
+
/**
- * @brief Returns the current time.
+ * @brief Get the current time suitable for Celix thread conditions.
+ *
+ * This function returns the current time compatible with the Celix thread conditions, specifically for
+ * the function celixThreadCondition_waitUntil, as long as the condition is initialized with
+ * celixThreadCondition_init.
*
- * The returned timespec can be used in celixThreadCondition_waitUntil and
- * will use a different clock depending on the OS (e.g. CLOCK_MONOTONIC or CLOCK_REALTIME).
- * The returned time is not meant to be used in logging the current time.
+ * Note: Do not use the returned time for logging or displaying the current time as the choice of clock
+ * varies based on the operating system.
*
- * @return The current time.
+ * @return A struct timespec denoting the current time.
*/
CELIX_UTILS_EXPORT struct timespec celixThreadCondition_getTime();
/**
- * @brief Returns the current time plus the given delayInSeconds.
+ * @brief Calculate the current time incremented by a given delay, suitable for Celix thread conditions.
*
- * The returned timespec can be used in celixThreadCondition_waitUntil and
- * will use a different clock depending on the OS (e.g. CLOCK_MONOTONIC or CLOCK_REALTIME).
- * The returned time is not meant to be used in logging the current time.
+ * This function provides the current time, increased by a specified delay (in seconds), compatible
+ * with Celix thread conditions. The resulting struct timespec can be used with the function
+ * celixThreadCondition_waitUntil, as long as the condition is initialized with celixThreadCondition_init.
*
- * @param delayInSeconds The delay in seconds to add to the current time.
- * @return The current time plus the given delayInSeconds.
+ * Note: Do not use the returned time for logging or displaying the current time as the choice of clock
+ * varies based on the operating system.
+ *
+ * @param[in] delayInSeconds The desired delay in seconds to be added to the current time.
+ * @return A struct timespec denoting the current time plus the provided delay.
*/
CELIX_UTILS_EXPORT struct timespec celixThreadCondition_getDelayedTime(double delayInSeconds);
+
/**
* @brief Wait for the condition to be signaled or until the given absolute time is reached.
*
diff --git a/libs/utils/src/celix_threads.c b/libs/utils/src/celix_threads.c
index 3916a5b8..8ba9cd63 100644
--- a/libs/utils/src/celix_threads.c
+++ b/libs/utils/src/celix_threads.c
@@ -16,18 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
-/**
- * celix_threads.c
- *
- * \date 4 Jun 2014
- * \author <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- * \copyright Apache License, Version 2.0
- */
#include <stdlib.h>
#include <sys/time.h>
#include <time.h>
-#include "signal.h"
+#include <signal.h>
+
#include "celix_threads.h"
#include "celix_utils.h"
@@ -150,11 +144,10 @@ celix_status_t celixThreadCondition_init(celix_thread_cond_t *condition, celix_t
return pthread_cond_init(condition, attr);
#else
celix_status_t status = CELIX_SUCCESS;
- if(attr) {
+ if (attr) {
status = pthread_condattr_setclock(attr, CLOCK_MONOTONIC);
status = CELIX_DO_IF(status, pthread_cond_init(condition, attr));
- }
- else {
+ } else {
celix_thread_condattr_t condattr;
(void)pthread_condattr_init(&condattr); // always return 0
status = pthread_condattr_setclock(&condattr, CLOCK_MONOTONIC);
@@ -188,17 +181,6 @@ celix_status_t celixThreadCondition_timedwaitRelative(celix_thread_cond_t *cond,
}
#endif
-CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_waitFor(celix_thread_cond_t* cond,
- celix_thread_mutex_t* mutex,
- double delayInSeconds) {
- if (delayInSeconds < 0) {
- return CELIX_ILLEGAL_ARGUMENT;
- }
- struct timespec now = celix_gettime(CLOCK_MONOTONIC);
- struct timespec absTime = celix_delayedTimespec(&now, delayInSeconds);
- return celixThreadCondition_waitUntil(cond, mutex, &absTime);
-}
-
struct timespec celixThreadCondition_getTime() {
return celixThreadCondition_getDelayedTime(0);
}
@@ -210,7 +192,7 @@ struct timespec celixThreadCondition_getDelayedTime(double delayInSeconds) {
gettimeofday(&tv, NULL);
TIMEVAL_TO_TIMESPEC(&tv, &now);
#else
- struct timespec now = celix_gettime(CLOCK_MONOTONIC);
+ struct timespec now = celix_gettime(CLOCK_REALTIME);
#endif
if (delayInSeconds == 0) {
return now;
@@ -218,10 +200,9 @@ struct timespec celixThreadCondition_getDelayedTime(double delayInSeconds) {
return celix_delayedTimespec(&now, delayInSeconds);
}
-CELIX_UTILS_EXPORT celix_status_t celixThreadCondition_waitUntil(celix_thread_cond_t* cond,
- celix_thread_mutex_t* mutex,
- const struct timespec* absTime) {
- if (absTime == NULL || absTime->tv_sec < 0 || absTime->tv_nsec < 0) {
+celix_status_t
+celixThreadCondition_waitUntil(celix_thread_cond_t* cond, celix_thread_mutex_t* mutex, const struct timespec* absTime) {
+ if (absTime == NULL) {
return CELIX_ILLEGAL_ARGUMENT;
}
return pthread_cond_timedwait(cond, mutex, absTime);
diff --git a/libs/utils/src/utils.c b/libs/utils/src/utils.c
index ed5cbfa4..24371d43 100644
--- a/libs/utils/src/utils.c
+++ b/libs/utils/src/utils.c
@@ -22,7 +22,6 @@
#include <string.h>
#include <assert.h>
#include <stdarg.h>
-#include <math.h>
#include "utils.h"
#include "celix_utils.h"