Eliminate deadlock condition in opendir()

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@757 42af7a65-404d-4744-a932-0658087f49c3

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 <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().
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 &lt;spudmonkey@racsa.co.cr&gt
 nuttx-0.3.11 2008-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
 
 	* 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 &lt;spudmonkey@racsa.co.cr&gt;
 
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 <spudmonkey@racsa.co.cr>
  *
  * 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 <nuttx/config.h>
 #include <sys/types.h>
@@ -48,29 +48,29 @@
 #include <nuttx/fs.h>
 #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 <spudmonkey@racsa.co.cr>
  *
  * 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 <nuttx/config.h>
 #include <sys/types.h>
@@ -43,34 +43,34 @@
 #include <nuttx/fs.h>
 #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 <spudmonkey@racsa.co.cr>
  *
  * 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 <nuttx/config.h>
 #include <sys/types.h>
@@ -44,34 +44,34 @@
 #include <nuttx/fs.h>
 #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 <nuttx/config.h>
+
 #include <sys/types.h>
 #include <stdlib.h>
 #include <dirent.h>
 #include <string.h>
+#include <assert.h>
 #include <errno.h>
+
 #include <nuttx/fs.h>
+
 #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);