posix: timer: use async pthread cancellation

include/zephyr/posix/signal.h

@@ -80,16 +80,16 @@ typedef struct {
 typedef int	sig_atomic_t;		/* Atomic entity type (ANSI) */
 
 union sigval {
-	int sival_int;
 	void *sival_ptr;
+	int sival_int;
 };
 
 struct sigevent {
-	int sigev_notify;
-	int sigev_signo;
-	union sigval sigev_value;
 	void (*sigev_notify_function)(union sigval val);
 	pthread_attr_t *sigev_notify_attributes;
+	union sigval sigev_value;
+	int sigev_notify;
+	int sigev_signo;
 };
 
 #ifdef CONFIG_POSIX_SIGNAL

lib/posix/timer.c

@@ -1,26 +1,27 @@
 /*
  * Copyright (c) 2018 Intel Corporation
+ * Copyright (c) 2024, Meta
  *
  * SPDX-License-Identifier: Apache-2.0
  */
-#include <zephyr/kernel.h>
 #include <errno.h>
-#include <string.h>
+
+#include <zephyr/kernel.h>
+#include <zephyr/logging/log.h>
 #include <zephyr/posix/pthread.h>
-#include <zephyr/sys/printk.h>
 #include <zephyr/posix/signal.h>
 #include <zephyr/posix/time.h>
 
 #define ACTIVE 1
 #define NOT_ACTIVE 0
 
+LOG_MODULE_REGISTER(posix_timer);
+
 static void zephyr_timer_wrapper(struct k_timer *ztimer);
 
 struct timer_obj {
 	struct k_timer ztimer;
-	void (*sigev_notify_function)(union sigval val);
-	union sigval val;
-	int sigev_notify;
+	struct sigevent evp;
 	struct k_sem sem_cond;
 	pthread_t thread;
 	struct timespec interval;	/* Reload value */
@@ -39,23 +40,53 @@ static void zephyr_timer_wrapper(struct k_timer *ztimer)
 
 	if (timer->reload == 0U) {
 		timer->status = NOT_ACTIVE;
+		LOG_DBG("timer %p not active", timer);
+		return;
 	}
 
-	(timer->sigev_notify_function)(timer->val);
+	if (timer->evp.sigev_notify == SIGEV_NONE) {
+		LOG_DBG("SIGEV_NONE");
+		return;
+	}
+
+	if (timer->evp.sigev_notify_function == NULL) {
+		LOG_DBG("NULL sigev_notify_function");
+		return;
+	}
+
+	LOG_DBG("calling sigev_notify_function %p", timer->evp.sigev_notify_function);
+	(timer->evp.sigev_notify_function)(timer->evp.sigev_value);
 }
 
 static void *zephyr_thread_wrapper(void *arg)
 {
+	int ret;
 	struct timer_obj *timer = (struct timer_obj *)arg;
 
+	ret = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+	__ASSERT(ret == 0, "pthread_setcanceltype() failed: %d", ret);
+
+	if (timer->evp.sigev_notify_attributes == NULL) {
+		ret = pthread_detach(pthread_self());
+		__ASSERT(ret == 0, "pthread_detach() failed: %d", ret);
+	}
+
 	while (1) {
 		if (timer->reload == 0U) {
 			timer->status = NOT_ACTIVE;
+			LOG_DBG("timer %p not active", timer);
 		}
 
-		k_sem_take(&timer->sem_cond, K_FOREVER);
+		ret = k_sem_take(&timer->sem_cond, K_FOREVER);
+		__ASSERT(ret == 0, "k_sem_take() failed: %d", ret);
+
+		if (timer->evp.sigev_notify_function == NULL) {
+			LOG_DBG("NULL sigev_notify_function");
+			continue;
+		}
 
-		(timer->sigev_notify_function)(timer->val);
+		LOG_DBG("calling sigev_notify_function %p", timer->evp.sigev_notify_function);
+		(timer->evp.sigev_notify_function)(timer->evp.sigev_value);
 	}
 
 	return NULL;
@@ -79,54 +110,89 @@ static void zephyr_timer_interrupt(struct k_timer *ztimer)
  */
 int timer_create(clockid_t clockid, struct sigevent *evp, timer_t *timerid)
 {
+	int ret = 0;
+	int detachstate;
 	struct timer_obj *timer;
 	const k_timeout_t alloc_timeout = K_MSEC(CONFIG_TIMER_CREATE_WAIT);
 
-	if (clockid != CLOCK_MONOTONIC || evp == NULL ||
-	    (evp->sigev_notify != SIGEV_NONE &&
-	     evp->sigev_notify != SIGEV_SIGNAL &&
-	     evp->sigev_notify != SIGEV_THREAD)) {
+	if (evp == NULL || timerid == NULL) {
 		errno = EINVAL;
 		return -1;
 	}
 
-	if (k_mem_slab_alloc(&posix_timer_slab, (void **)&timer, alloc_timeout) == 0) {
-		(void)memset(timer, 0, sizeof(struct timer_obj));
-	} else {
+	if (k_mem_slab_alloc(&posix_timer_slab, (void **)&timer, alloc_timeout) != 0) {
+		LOG_DBG("k_mem_slab_alloc() failed: %d", ret);
 		errno = ENOMEM;
 		return -1;
 	}
 
-	timer->sigev_notify_function = evp->sigev_notify_function;
-	timer->val = evp->sigev_value;
-	timer->interval.tv_sec = 0;
-	timer->interval.tv_nsec = 0;
-	timer->reload = 0U;
-	timer->status = NOT_ACTIVE;
+	*timer = (struct timer_obj){0};
+	timer->evp = *evp;
+	evp = &timer->evp;
 
-	if (evp->sigev_notify == SIGEV_NONE) {
+	switch (evp->sigev_notify) {
+	case SIGEV_NONE:
 		k_timer_init(&timer->ztimer, NULL, NULL);
-	} else if (evp->sigev_notify == SIGEV_THREAD) {
-		int ret;
+		break;
+	case SIGEV_SIGNAL:
+		k_timer_init(&timer->ztimer, zephyr_timer_wrapper, NULL);
+		break;
+	case SIGEV_THREAD:
+		if (evp->sigev_notify_attributes != NULL) {
+			ret = pthread_attr_getdetachstate(evp->sigev_notify_attributes,
+							  &detachstate);
+			if (ret != 0) {
+				LOG_DBG("pthread_attr_getdetachstate() failed: %d", ret);
+				errno = ret;
+				ret = -1;
+				goto free_timer;
+			}
+
+			if (detachstate != PTHREAD_CREATE_DETACHED) {
+				ret = pthread_attr_setdetachstate(evp->sigev_notify_attributes,
+								  PTHREAD_CREATE_DETACHED);
+				if (ret != 0) {
+					LOG_DBG("pthread_attr_setdetachstate() failed: %d", ret);
+					errno = ret;
+					ret = -1;
+					goto free_timer;
+				}
+			}
+		}
+
+		ret = k_sem_init(&timer->sem_cond, 0, 1);
+		if (ret != 0) {
+			LOG_DBG("k_sem_init() failed: %d", ret);
+			errno = -ret;
+			ret = -1;
+			goto free_timer;
+		}
 
-		timer->sigev_notify = SIGEV_THREAD;
-		k_sem_init(&timer->sem_cond, 0, 1);
 		ret = pthread_create(&timer->thread, evp->sigev_notify_attributes,
 							zephyr_thread_wrapper, timer);
 		if (ret != 0) {
-			k_mem_slab_free(&posix_timer_slab, (void *) &timer);
-			return ret;
+			LOG_DBG("pthread_create() failed: %d", ret);
+			errno = ret;
+			ret = -1;
+			goto free_timer;
 		}
 
-		pthread_detach(timer->thread);
 		k_timer_init(&timer->ztimer, zephyr_timer_interrupt, NULL);
-	} else {
-		k_timer_init(&timer->ztimer, zephyr_timer_wrapper, NULL);
+		break;
+	default:
+		ret = -1;
+		errno = EINVAL;
+		goto free_timer;
 	}
 
 	*timerid = (timer_t)timer;
+	goto out;
 
-	return 0;
+free_timer:
+	k_mem_slab_free(&posix_timer_slab, (void *)&timer);
+
+out:
+	return ret;
 }
 
 /**
@@ -266,8 +332,9 @@ int timer_delete(timer_t timerid)
 		k_timer_stop(&timer->ztimer);
 	}
 
-	if (timer->sigev_notify == SIGEV_THREAD)
-		pthread_cancel(timer->thread);
+	if (timer->evp.sigev_notify == SIGEV_THREAD) {
+		(void)pthread_cancel(timer->thread);
+	}
 
 	k_mem_slab_free(&posix_timer_slab, (void *)timer);
 

tests/posix/common/src/timer.c

@@ -30,7 +30,7 @@ void handler(union sigval val)
 	zassert_equal(val.sival_int, TEST_SIGNAL_VAL);
 }
 
-void test_timer(int sigev_notify)
+void test_timer(clockid_t clock_id, int sigev_notify)
 {
 	struct sigevent sig = {0};
 	struct itimerspec value, ovalue;
@@ -43,7 +43,7 @@ void test_timer(int sigev_notify)
 	sig.sigev_value.sival_int = TEST_SIGNAL_VAL;
 
 	/*TESTPOINT: Check if timer is created successfully*/
-	zassert_ok(timer_create(CLOCK_MONOTONIC, &sig, &timerid));
+	zassert_ok(timer_create(clock_id, &sig, &timerid));
 
 	value.it_value.tv_sec = DURATION_SECS;
 	value.it_value.tv_nsec = DURATION_NSECS;
@@ -59,9 +59,9 @@ void test_timer(int sigev_notify)
 	LOG_DBG("Time remaining to fire %d secs and  %d nsecs", (int)value.it_value.tv_sec,
 		(int)value.it_value.tv_nsec);
 
-	clock_gettime(CLOCK_MONOTONIC, &ts);
+	clock_gettime(clock_id, &ts);
 	sleep(SECS_TO_SLEEP);
-	clock_gettime(CLOCK_MONOTONIC, &te);
+	clock_gettime(clock_id, &te);
 
 	if (te.tv_nsec >= ts.tv_nsec) {
 		secs_elapsed = te.tv_sec - ts.tv_sec;
@@ -82,14 +82,24 @@ void test_timer(int sigev_notify)
 		       exp_count, expected_signal_count);
 }
 
-ZTEST(timer, test_SIGEV_SIGNAL)
+ZTEST(timer, test_CLOCK_REALTIME__SIGEV_SIGNAL)
+{
+	test_timer(CLOCK_REALTIME, SIGEV_SIGNAL);
+}
+
+ZTEST(timer, test_CLOCK_REALTIME__SIGEV_THREAD)
+{
+	test_timer(CLOCK_REALTIME, SIGEV_THREAD);
+}
+
+ZTEST(timer, test_CLOCK_MONOTONIC__SIGEV_SIGNAL)
 {
-	test_timer(SIGEV_SIGNAL);
+	test_timer(CLOCK_MONOTONIC, SIGEV_SIGNAL);
 }
 
-ZTEST(timer, test_SIGEV_THREAD)
+ZTEST(timer, test_CLOCK_MONOTONIC__SIGEV_THREAD)
 {
-	test_timer(SIGEV_THREAD);
+	test_timer(CLOCK_MONOTONIC, SIGEV_THREAD);
 }
 
 ZTEST(timer, test_timer_overrun)