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 <[email protected]> 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:[email protected]";>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"
