From ae5cc87281ae1463c64b6ebfbe46ef9f015a89af Mon Sep 17 00:00:00 2001 From: patacongo Date: Sat, 31 May 2008 18:33:44 +0000 Subject: [PATCH] Fix memory leak: Contained watchdog not being deleted with POSIX timer deleted git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@756 42af7a65-404d-4744-a932-0658087f49c3 --- ChangeLog | 2 ++ Documentation/NuttX.html | 2 ++ TODO | 6 ---- sched/timer_delete.c | 12 ++++---- sched/wd_cancel.c | 55 ++++++++++++++++++++--------------- sched/wd_delete.c | 63 ++++++++++++++++++++++++++-------------- 6 files changed, 83 insertions(+), 57 deletions(-) diff --git a/ChangeLog b/ChangeLog index a47f3effb6..975bbf1bbb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -358,3 +358,5 @@ 0.3.11 2008-xx-xx Gregory Nutt * Add support for recursive mutexes. + * BUGFIX: Eliminate a memory leak -- contained watchdog instance was not + being deleted with a POSIX timer was deleted. diff --git a/Documentation/NuttX.html b/Documentation/NuttX.html index 3a48481f2e..0ad73fcc2a 100644 --- a/Documentation/NuttX.html +++ b/Documentation/NuttX.html @@ -1008,6 +1008,8 @@ buildroot-0.1.0 2007-03-09 <spudmonkey@racsa.co.cr> nuttx-0.3.11 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> * Add support for recursive mutexes. + * BUGFIX: Eliminate a memory leak -- contained watchdog instance was not + being deleted with a POSIX timer was deleted. pascal-0.1.3 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> diff --git a/TODO b/TODO index a85852b4dd..aa390a918e 100644 --- a/TODO +++ b/TODO @@ -56,12 +56,6 @@ o Task/Scheduler (sched/) Priority: Medium, required for standard compliance (but makes the code bigger) - Description: Watchdog instance non-released by timer_delete() - Status: Open - Priority: High, if tasks use POSIX timers and delete them many - times, all of the pre-allocated watchdog timers get - lost. - o Dynamic Loader ^^^^^^^^^^^^^^ diff --git a/sched/timer_delete.c b/sched/timer_delete.c index 066e158a6e..3868075756 100644 --- a/sched/timer_delete.c +++ b/sched/timer_delete.c @@ -72,7 +72,7 @@ * ********************************************************************************/ -static void timer_free(struct posix_timer_s *timer) +static inline void timer_free(struct posix_timer_s *timer) { irqstate_t flags; @@ -137,10 +137,12 @@ int timer_delete(timer_t timerid) *get_errno_ptr() = EINVAL; return ERROR; } - - /* Disarm the timer */ - - (void)wd_cancel(timer->pt_wdog); + + /* 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 */ diff --git a/sched/wd_cancel.c b/sched/wd_cancel.c index 2872ce0521..0a22edd5d8 100644 --- a/sched/wd_cancel.c +++ b/sched/wd_cancel.c @@ -1,7 +1,7 @@ -/************************************************************ +/**************************************************************************** * wd_cancel.c * - * Copyright (C) 2007 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -14,7 +14,7 @@ * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. - * 3. Neither the name Gregory Nutt nor the names of its contributors may be + * 3. Neither the name NuttX nor the names of its contributors may be * used to endorse or promote products derived from this software * without specific prior written permission. * @@ -31,11 +31,11 @@ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. * - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Included Files - ************************************************************/ + ****************************************************************************/ #include #include @@ -44,31 +44,31 @@ #include "os_internal.h" #include "wd_internal.h" -/************************************************************ +/**************************************************************************** * Definitions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Type Declarations - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Global Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Function: wd_cancel * * Description: @@ -83,7 +83,7 @@ * * Assumptions: * - ************************************************************/ + ****************************************************************************/ STATUS wd_cancel (WDOG_ID wdid) { @@ -130,17 +130,24 @@ STATUS wd_cancel (WDOG_ID wdid) * is being canceled, then it inherits the remaining ticks. */ - if (curr->next) curr->next->lag += curr->lag; + if (curr->next) + { + curr->next->lag += curr->lag; + } /* Now, remove the watchdog from the timer queue */ if (prev) - (void)sq_remafter((FAR sq_entry_t*)prev, &g_wdactivelist); + { + (void)sq_remafter((FAR sq_entry_t*)prev, &g_wdactivelist); + } else - (void)sq_remfirst(&g_wdactivelist); + { + (void)sq_remfirst(&g_wdactivelist); + } wdid->next = NULL; - /* If we made it this far, then we were successful */ + /* Return success */ ret = OK; } diff --git a/sched/wd_delete.c b/sched/wd_delete.c index 7e2a4328e0..25f2db8abe 100644 --- a/sched/wd_delete.c +++ b/sched/wd_delete.c @@ -1,7 +1,7 @@ -/************************************************************ +/**************************************************************************** * wd_delete.c * - * Copyright (C) 2007 Gregory Nutt. All rights reserved. + * Copyright (C) 2007, 2008 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Redistribution and use in source and binary forms, with or without @@ -14,7 +14,7 @@ * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. - * 3. Neither the name Gregory Nutt nor the names of its contributors may be + * 3. Neither the name NuttX nor the names of its contributors may be * used to endorse or promote products derived from this software * without specific prior written permission. * @@ -31,43 +31,47 @@ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. * - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Included Files - ************************************************************/ + ****************************************************************************/ #include + #include #include +#include + #include + #include "wd_internal.h" -/************************************************************ +/**************************************************************************** * Definitions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Type Declarations - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Global Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Function: wd_delete * * Description: @@ -86,16 +90,31 @@ * The caller has assured that the watchdog is no longer * in use. * - ************************************************************/ + ****************************************************************************/ STATUS wd_delete(WDOG_ID wdId) { irqstate_t saved_state; - /* Check if the watchdog has been started. */ + /* Verify that a valid watchdog was provided */ + + if (!wdId) + { + *get_errno_ptr() = EINVAL; + return ERROR; + } + /* The following steps are atomic... the watchdog must not be active when + * it is being deallocated. + */ saved_state = irqsave(); - if (wdId->active) wd_cancel(wdId); + + /* Check if the watchdog has been started. */ + + if (wdId->active) + { + wd_cancel(wdId); + } /* Put the watchdog back on the free list */