Avoid calling pthread_join() to wait for USB MSC thread to terminate: This does not work if the caller of usb_mscuninitialize() is in a different task group than the MSC thread. From David Sidrane

2 files changed, 39 insertions, 1 deletions

diff --git a/nuttx/drivers/usbdev/usbmsc.c b/nuttx/drivers/usbdev/usbmsc.c
index 5f1b86802..710b3ba73 100644
--- a/nuttx/drivers/usbdev/usbmsc.c
+++ b/nuttx/drivers/usbdev/usbmsc.c
@@ -1628,6 +1628,17 @@ int usbmsc_exportluns(FAR void *handle)
       goto errout_with_mutex;
     }
 
+  /* Detach the pthread so that we do not create a memory leak.
+   *
+   * REVISIT:  See related comments in usbmsc_uninitialize()
+   */
+
+  ret = pthread_detach(priv->thread);
+  if (ret != OK)
+    {
+      usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_DETACH), (uint16_t)-ret);
+    }
+
   /* Register the USB storage class driver (unless we are part of a composite device) */
 
 #ifndef CONFIG_USBMSC_COMPOSITE
@@ -1752,7 +1763,33 @@ void usbmsc_uninitialize(FAR void *handle)
        * garbage
        */
 
+#if 0
+      /* REVISIT:  pthread_join does not work in all contexts.  In
+       * particular, if usbmsc_uninitialize() executes in a different
+       * task group than the group that includes the SCSI thread, then
+       * pthread_join will fail.
+       *
+       * NOTE: If, for some reason, you wanted to restore this code,
+       * there is now a matching pthread_detach() elsewhere to prevent
+       * memory leaks.
+       */
+
       (void)pthread_join(priv->thread, &value);
+
+#else
+      /* REVISIT:  Calling pthread_mutex_lock and pthread_cond_wait
+       * from outside of the task group is equally non-standard.
+       * However, this actually works.
+       */
+
+      pthread_mutex_lock(&priv->mutex);
+      while ((priv->theventset & USBMSC_EVENT_TERMINATEREQUEST) != 0)
+        {
+          pthread_cond_wait(&priv->cond, &priv->mutex);
+        }
+
+      pthread_mutex_unlock(&priv->mutex);
+#endif
     }
 
   priv->thread = 0;
diff --git a/nuttx/drivers/usbdev/usbmsc_scsi.c b/nuttx/drivers/usbdev/usbmsc_scsi.c
index 7791141d8..44475311b 100644
--- a/nuttx/drivers/usbdev/usbmsc_scsi.c
+++ b/nuttx/drivers/usbdev/usbmsc_scsi.c
@@ -85,7 +85,7 @@
  * "seems to relate to stalling the endpoint when a short response is
  * generated which causes a residue to exist when the CSW would be returned.
  * I think there's two issues here.  The first being if the transfer which
- * had just been queued before the stall had not completed then it wouldn’t
+ * had just been queued before the stall had not completed then it wouldn't
  * then complete once the endpoint was stalled?  The second is that the
  * subsequent transfer for the CSW would be dropped on the floor (by the
  * epsubmit() function) if the end point was still stalled as the control
@@ -2678,5 +2678,6 @@ void *usbmsc_workerthread(void *arg)
   /* Transition to the TERMINATED state and exit */
 
   priv->thstate = USBMSC_STATE_TERMINATED;
+  pthread_cond_signal(&priv->cond);             /* See comments in usbmsc_uninitialize() */
   return NULL;
 }