diff --git a/ChangeLog b/ChangeLog index 28c4a0802cc..9e3686783bc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -370,3 +370,5 @@ * Add a ramdisk block driver 0.3.12 2008-xx-xx Gregory Nutt + + * Improved solution to POSIX timer lifetime controls bug fixed in 0.3.11. diff --git a/sched/timer_create.c b/sched/timer_create.c index a9e19dc2ade..a616e35215a 100644 --- a/sched/timer_create.c +++ b/sched/timer_create.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include "timer_internal.h" @@ -203,6 +204,7 @@ int timer_create(clockid_t clockid, FAR struct sigevent *evp, FAR timer_t *timer /* Initialize the timer instance */ + ret->pt_crefs = 1; ret->pt_owner = getpid(); ret->pt_delay = 0; ret->pt_wdog = wdog; diff --git a/sched/timer_delete.c b/sched/timer_delete.c index e6afc57bf20..662455f0182 100644 --- a/sched/timer_delete.c +++ b/sched/timer_delete.c @@ -103,6 +103,53 @@ static inline void timer_free(struct posix_timer_s *timer) * Public Functions ********************************************************************************/ +/******************************************************************************** + * Function: timer_release + * + * Description: + * timer_release implements the heart of timer_delete. It is private to the + * the OS internals and differs only in that return value of 1 means that the + * timer was not actually deleted. + * + * Parameters: + * timer - The per-thread timer, previously created by the call to + * timer_create(), to be deleted. + * + * Return Value: + * If the call succeeds, timer_release() will return 0 (OK) or 1 (meaning that + * the timer is still valid). Otherwise, the function will return a negated errno: + * + * -EINVAL - The timer specified timerid is not valid. + * + ********************************************************************************/ + +int timer_release(FAR struct posix_timer_s *timer) +{ + /* Some sanity checks */ + + if (!timer) + { + return -EINVAL; + } + + if (timer->pt_crefs > 1) + { + timer->pt_crefs--; + return 1; + } + + /* Free the underlying watchdog instance (the timer will be canceled by the + * watchdog logic before it is actually deleted) + */ + + (void)wd_delete(timer->pt_wdog); + + /* Release the timer structure */ + + timer_free(timer); + return OK; +} + /******************************************************************************** * Function: timer_delete * @@ -113,7 +160,7 @@ static inline void timer_free(struct posix_timer_s *timer) * removal. The disposition of pending signals for the deleted timer is unspecified. * * Parameters: - * timerid - The pre-thread timer, previously created by the call to + * timerid - The per-thread timer, previously created by the call to * timer_create(), to be deleted. * * Return Value: @@ -128,33 +175,11 @@ static inline void timer_free(struct posix_timer_s *timer) int timer_delete(timer_t timerid) { - FAR struct posix_timer_s *timer = (FAR struct posix_timer_s *)timerid; - - /* Some sanity checks */ - - if (!timer) + int ret = timer_release((FAR struct posix_timer_s *)timerid); + if (ret < 0) { - *get_errno_ptr() = EINVAL; - return ERROR; - } - - /* If the watchdog structure is busy now, then just mark it for deletion later */ - - if ((timer->pt_flags & PT_FLAGS_BUSY) != 0) - { - timer->pt_flags |= PT_FLAGS_DELETED; - } - else - { - /* Free the underlying watchdog instance (the timer will be canceled by the - * watchdog logic before it is actually deleted) - */ - - (void)wd_delete(timer->pt_wdog); - - /* Release the timer structure */ - - timer_free(timer); + *get_errno_ptr() = -ret; + return ERROR; } return OK; } diff --git a/sched/timer_internal.h b/sched/timer_internal.h index e4403499cb9..39946a8d32e 100644 --- a/sched/timer_internal.h +++ b/sched/timer_internal.h @@ -50,11 +50,6 @@ ********************************************************************************/ #define PT_FLAGS_PREALLOCATED 0x01 /* Timer comes from a pool of preallocated timers */ -#define PT_FLAGS_BUSY 0x02 /* Timer cannot be deleted now */ -#define PT_FLAGS_DELETED 0x04 /* Busy timer marked for deletion */ - -#define PT_FLAGS_STATIC PT_FLAGS_PREALLOCATED -#define PT_FLAGS_DYNAMIC (~PT_FLAGS_STATIC) /******************************************************************************** * Public Types @@ -67,6 +62,7 @@ struct posix_timer_s FAR struct posix_timer_s *flink; ubyte pt_flags; /* See PT_FLAGS_* definitions */ + ubyte pt_crefs; /* Reference count */ ubyte pt_signo; /* Notification signal */ pid_t pt_owner; /* Creator of timer */ int pt_delay; /* If non-zero, used to reset repetitive timers */ @@ -98,5 +94,6 @@ extern volatile sq_queue_t g_alloctimers; extern void weak_function timer_initialize(void); extern void weak_function timer_deleteall(pid_t pid); +extern int timer_release(FAR struct posix_timer_s *timer); #endif /* __TIMER_INTERNAL_H */ diff --git a/sched/timer_settime.c b/sched/timer_settime.c index ab130210e3f..2068f56abb0 100644 --- a/sched/timer_settime.c +++ b/sched/timer_settime.c @@ -181,21 +181,19 @@ static void timer_timeout(int argc, uint32 itimer) u.itimer = itimer; - /* Send the specified signal to the specified task. */ + /* Send the specified signal to the specified task. Increment the reference + * count on the timer first so that will not be deleted until after the + * signal handler returns. + */ - u.timer->pt_flags |= PT_FLAGS_BUSY; + u.timer->pt_crefs++; timer_sigqueue(u.timer); - u.timer->pt_flags &= ~PT_FLAGS_BUSY; - /* Check if the signal handler attempted to delete the timer */ + /* Release the reference. timer_release will return nonzero if the timer + * was not deleted. + */ - if ((u.timer->pt_flags & PT_FLAGS_DELETED) != 0) - { - /* Yes.. delete the timer now that we are no longer busy */ - - timer_delete(u.timer); - } - else + if (timer_release(u.timer)) { /* If this is a repetitive timer, the restart the watchdog */ @@ -204,21 +202,19 @@ static void timer_timeout(int argc, uint32 itimer) #else FAR struct posix_timer_s *timer = (FAR struct posix_timer_s *)itimer; - /* Send the specified signal to the specified task. */ + /* Send the specified signal to the specified task. Increment the reference + * count on the timer first so that will not be deleted until after the + * signal handler returns. + */ - timer->pt_flags |= PT_FLAGS_BUSY; + timer->pt_crefs++; timer_sigqueue(timer); - timer->pt_flags &= ~PT_FLAGS_BUSY; - /* Check if the signal handler attempted to delete the timer */ + /* Release the reference. timer_release will return nonzero if the timer + * was not deleted. + */ - if ((timer->pt_flags & PT_FLAGS_DELETED) != 0) - { - /* Yes.. delete the timer now that we are no longer busy */ - - timer_delete(timer); - } - else + if (timer_release(timer)) { /* If this is a repetitive timer, the restart the watchdog */