From bd6c6f423d88aa5ae35b3e833cb96da83a878d8b Mon Sep 17 00:00:00 2001 From: patacongo Date: Sat, 31 May 2008 20:14:15 +0000 Subject: Eliminate deadlock condition in opendir() git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@757 42af7a65-404d-4744-a932-0658087f49c3 --- nuttx/ChangeLog | 3 ++- nuttx/Documentation/NuttX.html | 3 ++- nuttx/fs/fs_inode.c | 60 +++++++++++++++++++++--------------------- nuttx/fs/fs_inodeaddref.c | 36 ++++++++++++------------- nuttx/fs/fs_inoderelease.c | 36 ++++++++++++------------- nuttx/fs/fs_opendir.c | 46 ++++++++++++++++++++------------ 6 files changed, 99 insertions(+), 85 deletions(-) diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index 975bbf1bb..180762c8b 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -358,5 +358,6 @@ 0.3.11 2008-xx-xx Gregory Nutt * Add support for recursive mutexes. - * BUGFIX: Eliminate a memory leak -- contained watchdog instance was not + * BUGFIX: Eliminate a memory leak -- contained watchdog instance was not being deleted with a POSIX timer was deleted. + * BUGFIX: Eliminate a deadlock condition in opendir(). diff --git a/nuttx/Documentation/NuttX.html b/nuttx/Documentation/NuttX.html index 0ad73fcc2..3efaa0ebf 100644 --- a/nuttx/Documentation/NuttX.html +++ b/nuttx/Documentation/NuttX.html @@ -1008,8 +1008,9 @@ 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 + * BUGFIX: Eliminate a memory leak -- contained watchdog instance was not being deleted with a POSIX timer was deleted. + * BUGFIX: Eliminate a deadlock condition in opendir(). pascal-0.1.3 2008-xx-xx Gregory Nutt <spudmonkey@racsa.co.cr> diff --git a/nuttx/fs/fs_inode.c b/nuttx/fs/fs_inode.c index 73a8601b3..d241fd42b 100644 --- a/nuttx/fs/fs_inode.c +++ b/nuttx/fs/fs_inode.c @@ -1,7 +1,7 @@ -/************************************************************ +/**************************************************************************** * fs_inode.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 @@ -48,29 +48,29 @@ #include #include "fs_internal.h" -/************************************************************ +/**************************************************************************** * Definitions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ static sem_t tree_sem; -/************************************************************ +/**************************************************************************** * Public Variables - ************************************************************/ + ****************************************************************************/ FAR struct inode *root_inode = NULL; -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Name: _inode_compare - ************************************************************/ + ****************************************************************************/ static int _inode_compare(const char *fname, FAR struct inode *node) @@ -135,18 +135,18 @@ static int _inode_compare(const char *fname, } } -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Name: fs_initialize * * Description: * This is called from the OS initialization logic to configure * the file system. * - ************************************************************/ + ****************************************************************************/ void fs_initialize(void) { @@ -166,9 +166,9 @@ void fs_initialize(void) } } -/************************************************************ +/**************************************************************************** * Name: inode_semtake - ************************************************************/ + ****************************************************************************/ void inode_semtake(void) { @@ -184,16 +184,16 @@ void inode_semtake(void) } } -/************************************************************ +/**************************************************************************** * Name: inode_semgive - ************************************************************/ + ****************************************************************************/ void inode_semgive(void) { sem_post(&tree_sem); } -/************************************************************ +/**************************************************************************** * Name: inode_search * * Description: @@ -203,7 +203,7 @@ void inode_semgive(void) * Assumptions: * The caller holds the tree_sem * - ************************************************************/ + ****************************************************************************/ FAR struct inode *inode_search(const char **path, FAR struct inode **peer, @@ -299,9 +299,9 @@ FAR struct inode *inode_search(const char **path, return node; } -/************************************************************ +/**************************************************************************** * Name: inode_free - ************************************************************/ + ****************************************************************************/ void inode_free(FAR struct inode *node) { @@ -313,14 +313,14 @@ void inode_free(FAR struct inode *node) } } -/************************************************************ +/**************************************************************************** * Name: inode_nextname * * Description: * Given a path with node names separated by '/', return * the next node name. * - ************************************************************/ + ****************************************************************************/ const char *inode_nextname(const char *name) { diff --git a/nuttx/fs/fs_inodeaddref.c b/nuttx/fs/fs_inodeaddref.c index 278cf08ab..584c7b70b 100644 --- a/nuttx/fs/fs_inodeaddref.c +++ b/nuttx/fs/fs_inodeaddref.c @@ -1,7 +1,7 @@ -/************************************************************ +/**************************************************************************** * fs_inodeaddref.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 @@ -43,34 +43,34 @@ #include #include "fs_internal.h" -/************************************************************ +/**************************************************************************** * Definitions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Name: inode_addref * * Description: * Increment the reference count on an inode (as when a file * descriptor is dup'ed. * - ************************************************************/ + ****************************************************************************/ void inode_addref(FAR struct inode *inode) { diff --git a/nuttx/fs/fs_inoderelease.c b/nuttx/fs/fs_inoderelease.c index 73ca72603..b37b0100f 100644 --- a/nuttx/fs/fs_inoderelease.c +++ b/nuttx/fs/fs_inoderelease.c @@ -1,7 +1,7 @@ -/************************************************************ +/**************************************************************************** * fs_inoderelease.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,34 +44,34 @@ #include #include "fs_internal.h" -/************************************************************ +/**************************************************************************** * Definitions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Variables - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Private Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Public Functions - ************************************************************/ + ****************************************************************************/ -/************************************************************ +/**************************************************************************** * Name: inode_release * * Description: * This is called from close() logic when it no longer refers * to the inode. * - ************************************************************/ + ****************************************************************************/ void inode_release(FAR struct inode *node) { diff --git a/nuttx/fs/fs_opendir.c b/nuttx/fs/fs_opendir.c index 2de8107d7..969b1d164 100644 --- a/nuttx/fs/fs_opendir.c +++ b/nuttx/fs/fs_opendir.c @@ -38,12 +38,16 @@ ****************************************************************************/ #include + #include #include #include #include +#include #include + #include + #include "fs_internal.h" /**************************************************************************** @@ -152,13 +156,13 @@ FAR DIR *opendir(FAR const char *path) if (INODE_IS_MOUNTPT(inode)) { /* Yes, then return the inode itself as the 'root' of - * the directory. The actually directory is at relpath into the - * mounted filesystem. - */ + * the directory. The actually directory is at relpath into the + * mounted filesystem. + */ /* The node is a file system mointpoint. Verify that the mountpoint - * supports the opendir() method - */ + * supports the opendir() method + */ if (!inode->u.i_mops || !inode->u.i_mops->opendir) { @@ -167,8 +171,8 @@ FAR DIR *opendir(FAR const char *path) } /* Take reference to the mountpoint inode (fd_root). Note that we do - * not use inode_addref() because we already hold the tree semaphore. - */ + * not use inode_addref() because we already hold the tree semaphore. + */ inode->i_crefs++; @@ -177,8 +181,21 @@ FAR DIR *opendir(FAR const char *path) ret = inode->u.i_mops->opendir(inode, relpath, dir); if (ret < 0) { + /* We now need to back off our reference to the inode. We can't + * call inode_release() to do that unless we release the tree + * semaphore. The following should be safe because: (1) after the + * reference count was incremented above it should be >1 so it should + * not decrement to zero, and (2) we hold the tree semaphore so no + * other thread should be able to change the reference count. + */ + + inode->i_crefs--; + DEBUGASSERT(inode->i_crefs > 0); + + /* Negate the error value so that it can be used to set errno */ + ret = -ret; - goto errout_with_inode; + goto errout_with_direntry; } } else @@ -200,10 +217,10 @@ FAR DIR *opendir(FAR const char *path) } /* It looks we have a valid psuedo-filesystem node. Take two references - * on the inode -- one for the parent (fd_root) and one for the child (fd_next). - * Note that we do not call inode_addref because we are holding - * the tree semaphore and that would result in deadlock. - */ + * on the inode -- one for the parent (fd_root) and one for the child (fd_next). + * Note that we do not call inode_addref because we are holding + * the tree semaphore and that would result in deadlock. + */ inode->i_crefs += 2; dir->u.psuedo.fd_next = inode; /* This is the next node to use for readdir() */ @@ -219,11 +236,6 @@ FAR DIR *opendir(FAR const char *path) /* Nasty goto's make error handling simpler */ -#ifndef CONFIG_DISABLE_MOUNTPOINT -errout_with_inode: - inode_release(inode); -#endif - errout_with_direntry: free(dir); -- cgit v1.2.3