diff options
-rw-r--r-- | nuttx/ChangeLog | 2 | ||||
-rw-r--r-- | nuttx/sched/timer_create.c | 2 | ||||
-rw-r--r-- | nuttx/sched/timer_delete.c | 79 | ||||
-rw-r--r-- | nuttx/sched/timer_internal.h | 7 | ||||
-rw-r--r-- | nuttx/sched/timer_settime.c | 40 |
5 files changed, 76 insertions, 54 deletions
diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 28c4a0802..9e3686783 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -370,3 +370,5 @@ * Add a ramdisk block driver 0.3.12 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> + + * Improved solution to POSIX timer lifetime controls bug fixed in 0.3.11. diff --git a/nuttx/sched/timer_create.c b/nuttx/sched/timer_create.c index a9e19dc2a..a616e3521 100644 --- a/nuttx/sched/timer_create.c +++ b/nuttx/sched/timer_create.c @@ -41,6 +41,7 @@ #include <stdlib.h> #include <unistd.h> #include <time.h> +#include <string.h> #include <wdog.h> #include <errno.h> #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/nuttx/sched/timer_delete.c b/nuttx/sched/timer_delete.c index e6afc57bf..662455f01 100644 --- a/nuttx/sched/timer_delete.c +++ b/nuttx/sched/timer_delete.c @@ -104,6 +104,53 @@ static inline void timer_free(struct posix_timer_s *timer) ********************************************************************************/ /******************************************************************************** + * 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 * * Description: @@ -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) - { - *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) + int ret = timer_release((FAR struct posix_timer_s *)timerid); + if (ret < 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/nuttx/sched/timer_internal.h b/nuttx/sched/timer_internal.h index e4403499c..39946a8d3 100644 --- a/nuttx/sched/timer_internal.h +++ b/nuttx/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/nuttx/sched/timer_settime.c b/nuttx/sched/timer_settime.c index ab130210e..2068f56ab 100644 --- a/nuttx/sched/timer_settime.c +++ b/nuttx/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 */ - if ((u.timer->pt_flags & PT_FLAGS_DELETED) != 0) - { - /* Yes.. delete the timer now that we are no longer busy */ + /* Release the reference. timer_release will return nonzero if the timer + * was not deleted. + */ - 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 */ - if ((timer->pt_flags & PT_FLAGS_DELETED) != 0) - { - /* Yes.. delete the timer now that we are no longer busy */ + /* Release the reference. timer_release will return nonzero if the timer + * was not deleted. + */ - timer_delete(timer); - } - else + if (timer_release(timer)) { /* If this is a repetitive timer, the restart the watchdog */ |