Fix a deadlock when using the NSH ifconfig command over Telnet

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

ChangeLog

@@ -2563,3 +2563,11 @@
 	  for use within the OS.
 	* arch/mips/src/pic32mx/pic32mx_ethernet.c:  The PIC32 Ethernet driver
 	  is now stable on the PIC32 Starter Kit.
+	* configs/pic32-starterkit/nsh2:  Add a PIC32 Ethernet Starter Kit NSH
+	  configuration that has no serial console; all interaction is done via
+	  Telnet.
+	* net/netdev_sem.c:  Correct a deadlock condition by making a seamphore
+	  recursive.  To my knowledge this deadlock only occurs when running the
+	  NSH command ifconfig over Telnet.  In that case the function netdev_foreach
+	  takes the network device semaphore, but so does the telnet logic causing
+	  the deadlock.

net/Makefile

@@ -1,8 +1,8 @@
 ############################################################################
 # net/Makefile
 #
-#   Copyright (C) 2007, 2008, 2011 Gregory Nutt. All rights reserved.
+#   Copyright (C) 2007, 2008, 2011-2012 Gregory Nutt. All rights reserved.
-#   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+#   Author: Gregory Nutt <gnutt@nuttx.org>
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -73,7 +73,7 @@ endif
 NETDEV_ASRCS =
 NETDEV_CSRCS = netdev_register.c netdev_ioctl.c net_poll.c netdev_txnotify.c \
 		  netdev_findbyname.c netdev_findbyaddr.c netdev_count.c \
-		  netdev_foreach.c netdev_unregister.c
+		  netdev_foreach.c netdev_unregister.c netdev_sem.c
 
 include uip/Make.defs
 endif

net/net_internal.h

@@ -143,7 +143,6 @@
 
 #if CONFIG_NSOCKET_DESCRIPTORS > 0
 extern struct uip_driver_s *g_netdevices;
-extern sem_t                g_netdev_sem;
 #endif
 
 /****************************************************************************
@@ -187,8 +186,9 @@ EXTERN void net_dsec2timeval(uint16_t dsec, struct timeval *tv);
 /* net_register.c ************************************************************/
 
 #if CONFIG_NSOCKET_DESCRIPTORS > 0
+EXTERN void netdev_seminit(void);
 EXTERN void netdev_semtake(void);
-# define netdev_semgive() sem_post(&g_netdev_sem)
+EXTERN void netdev_semgive(void);
 #endif
 
 /* net_findbyname.c **********************************************************/

net/net_sockets.c

@@ -106,7 +106,7 @@ void net_initialize(void)
   /* Initialize the socket layer */
 
 #if CONFIG_NSOCKET_DESCRIPTORS > 0
-  sem_init(&g_netdev_sem, 0, 1);
+  netdev_seminit();
 #endif
 
   /* Initialize the periodic ARP timer */

net/netdev_register.c

@@ -1,8 +1,8 @@
 /****************************************************************************
  * net/netdev_register.c
  *
- *   Copyright (C) 2007-2011 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2012 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -79,8 +79,8 @@ static int g_next_devnum = 0;
  ****************************************************************************/
 
 /* List of registered ethernet device drivers */
+
 struct uip_driver_s *g_netdevices = NULL;
-sem_t                g_netdev_sem;
 
 /****************************************************************************
  * Private Functions
@@ -90,28 +90,6 @@ sem_t                g_netdev_sem;
  * Global Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Function: netdev_semtake
- *
- * Description:
- *   Managed access to the network device list
- *
- ****************************************************************************/
-
-void netdev_semtake(void)
-{
-  /* Take the semaphore (perhaps waiting) */
-
-  while (uip_lockedwait(&g_netdev_sem) != 0)
-    {
-      /* The only case that an error should occur here is if
-       * the wait was awakened by a signal.
-       */
-
-      ASSERT(*get_errno_ptr() == EINTR);
-    }
-}
-
 /****************************************************************************
  * Function: netdev_register
  *

net/netdev_sem.c

@@ -0,0 +1,178 @@
+/****************************************************************************
+ * net/netdev_sem.c
+ *
+ *   Copyright (C) 2012 Gregory Nutt. All rights reserved.
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 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.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#if defined(CONFIG_NET) && CONFIG_NSOCKET_DESCRIPTORS > 0
+
+#include <sys/types.h>
+
+#include <unistd.h>
+#include <semaphore.h>
+#include <assert.h>
+#include <errno.h>
+
+#include "net_internal.h"
+
+/****************************************************************************
+ * Definitions
+ ****************************************************************************/
+
+#define NO_HOLDER (pid_t)-1
+
+/****************************************************************************
+ * Priviate Types
+ ****************************************************************************/
+
+/* There is at least on context in which recursive semaphores are required:
+ * When netdev_foreach is used with a telnet client, we will deadlock if we
+ * do not provide this capability.
+ */
+
+struct netdev_sem_s
+{
+  sem_t        sem;
+  pid_t        holder;
+  unsigned int count;
+};
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+static struct netdev_sem_s g_devlock;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Global Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Function: netdev_seminit
+ *
+ * Description:
+ *   Initialize the network device semaphore.
+ *
+ ****************************************************************************/
+
+void netdev_seminit(void)
+{
+  sem_init(&g_devlock.sem, 0, 1);
+  g_devlock.holder = NO_HOLDER;
+  g_devlock.count  = 0;
+}
+
+/****************************************************************************
+ * Function: netdev_semtake
+ *
+ * Description:
+ *   Get exclusive access to the network device list.
+ *
+ ****************************************************************************/
+
+void netdev_semtake(void)
+{
+  pid_t me = getpid();
+
+  /* Does this thread already hold the semaphore? */
+
+  if (g_devlock.holder == me)
+    {
+      /* Yes.. just increment the reference count */
+
+      g_devlock.count++;
+    }
+  else
+    {
+      /* No.. take the semaphore (perhaps waiting) */
+
+      while (uip_lockedwait(&g_devlock.sem) != 0)
+        {
+          /* The only case that an error should occur here is if
+           * the wait was awakened by a signal.
+           */
+
+          ASSERT(errno == EINTR);
+        }
+
+      /* Now this thread holds the semaphore */
+
+      g_devlock.holder = me;
+      g_devlock.count  = 1;
+    }
+}
+
+/****************************************************************************
+ * Function: netdev_semtake
+ *
+ * Description:
+ *   Release exclusive access to the network device list
+ *
+ ****************************************************************************/
+
+void netdev_semgive(void)
+{
+  DEBUGASSERT(g_devlock.holder == getpid() && g_devlock.count > 0);
+
+  /* If the count would go to zero, then release the semaphore */
+
+  if (g_devlock.count == 1)
+    {
+      /* We no longer hold the semaphore */
+
+      g_devlock.holder = NO_HOLDER;
+      g_devlock.count  = 0;
+      sem_post(&g_devlock.sem);
+    }
+  else
+    {
+      /* We still hold the semaphore. Just decrement the count */
+
+      g_devlock.count--;
+    }
+}
+
+#endif /* CONFIG_NET && CONFIG_NSOCKET_DESCRIPTORS */

net/uip/uip_lock.c

@@ -1,8 +1,8 @@
 /****************************************************************************
  * net/uip/uip_lock.c
  *
- *   Copyright (C) 2011 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2011-2012 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -173,7 +173,7 @@ void uip_unlock(uip_lock_t flags)
  * Function: uip_lockedwait
  *
  * Description:
- *   Atomically wait for sem while temporarilty releasing.
+ *   Atomically wait for sem while temporarily releasing g_uipsem.
  *
  ****************************************************************************/
 
@@ -195,7 +195,7 @@ int uip_lockedwait(sem_t *sem)
       g_count  = 0;
       sem_post(&g_uipsem);
 
-      /* Now take semaphore */
+      /* Now take the semaphore */
 
       ret = sem_wait(sem);