diff --git a/ChangeLog b/ChangeLog index 7b6d5a05d59..c129d79e85c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5938,3 +5938,9 @@ * fs/fat/fs_fat32util.c: In one error return case, the error return value was not being set, making the failure look like success. From David Sidrane (2011-10-1). + * drivers/usbdev/usbmsc.c and usbmsc_scsi.c: pthread_join() does not + work if called from a different task group than the pthread. This + is correct behavior, but a problem. The correct solution would be + configure the USB MSC thread to a task, however, this workaround + from David Sidrane plugs the hole for now (2013-11-1). + diff --git a/drivers/usbdev/usbmsc.c b/drivers/usbdev/usbmsc.c index 5f1b8680225..710b3ba73eb 100644 --- a/drivers/usbdev/usbmsc.c +++ b/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/drivers/usbdev/usbmsc_scsi.c b/drivers/usbdev/usbmsc_scsi.c index 7791141d804..44475311bdb 100644 --- a/drivers/usbdev/usbmsc_scsi.c +++ b/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; } diff --git a/include/nuttx/usb/usbdev_trace.h b/include/nuttx/usb/usbdev_trace.h index 5a5d4067046..2e210ac0729 100644 --- a/include/nuttx/usb/usbdev_trace.h +++ b/include/nuttx/usb/usbdev_trace.h @@ -365,33 +365,34 @@ #define USBMSC_TRACEERR_SNDSTATUSSUBMIT 0x00da #define USBMSC_TRACEERR_SYNCCACHEMEDIANOTPRESENT 0x00db #define USBMSC_TRACEERR_THREADCREATE 0x00dc -#define USBMSC_TRACEERR_TOOMANYLUNS 0x00dd -#define USBMSC_TRACEERR_UNBINDINVALIDARGS 0x00de -#define USBMSC_TRACEERR_UNBINDLUNINVALIDARGS1 0x00df -#define USBMSC_TRACEERR_UNBINDLUNINVALIDARGS2 0x00e0 -#define USBMSC_TRACEERR_UNINITIALIZEINVALIDARGS 0x00e1 -#define USBMSC_TRACEERR_UNSUPPORTEDSTDREQ 0x00e2 -#define USBMSC_TRACEERR_VERIFY10FLAGS 0x00e3 -#define USBMSC_TRACEERR_VERIFY10LBARANGE 0x00e4 -#define USBMSC_TRACEERR_VERIFY10MEDIANOTPRESENT 0x00e5 -#define USBMSC_TRACEERR_VERIFY10NOBLOCKS 0x00e6 -#define USBMSC_TRACEERR_VERIFY10READFAIL 0x00e7 -#define USBMSC_TRACEERR_WRALLOCREQ 0x00e8 -#define USBMSC_TRACEERR_WRCOMPLETEINVALIDARGS 0x00e9 -#define USBMSC_TRACEERR_WRITE10FLAGS 0x00ea -#define USBMSC_TRACEERR_WRITE10LBARANGE 0x00eb -#define USBMSC_TRACEERR_WRITE10MEDIANOTPRESENT 0x00ec -#define USBMSC_TRACEERR_WRITE10READONLY 0x00ed -#define USBMSC_TRACEERR_WRITE12FLAGS 0x00ee -#define USBMSC_TRACEERR_WRITE12LBARANGE 0x00ef -#define USBMSC_TRACEERR_WRITE12MEDIANOTPRESENT 0x00f0 -#define USBMSC_TRACEERR_WRITE12READONLY 0x00f1 -#define USBMSC_TRACEERR_WRITE6LBARANGE 0x00f2 -#define USBMSC_TRACEERR_WRITE6MEDIANOTPRESENT 0x00f3 -#define USBMSC_TRACEERR_WRITE6READONLY 0x00f4 -#define USBMSC_TRACEERR_WRSHUTDOWN 0x00f5 -#define USBMSC_TRACEERR_WRUNEXPECTED 0x00f6 -#define USBMSC_TRACEERR_UNSUPPORTEDTYPE 0x00f7 +#define USBMSC_TRACEERR_DETACH 0x00dd +#define USBMSC_TRACEERR_TOOMANYLUNS 0x00de +#define USBMSC_TRACEERR_UNBINDINVALIDARGS 0x00df +#define USBMSC_TRACEERR_UNBINDLUNINVALIDARGS1 0x00e0 +#define USBMSC_TRACEERR_UNBINDLUNINVALIDARGS2 0x00e1 +#define USBMSC_TRACEERR_UNINITIALIZEINVALIDARGS 0x00ee +#define USBMSC_TRACEERR_UNSUPPORTEDSTDREQ 0x00e3 +#define USBMSC_TRACEERR_VERIFY10FLAGS 0x00e4 +#define USBMSC_TRACEERR_VERIFY10LBARANGE 0x00e5 +#define USBMSC_TRACEERR_VERIFY10MEDIANOTPRESENT 0x00e6 +#define USBMSC_TRACEERR_VERIFY10NOBLOCKS 0x00e7 +#define USBMSC_TRACEERR_VERIFY10READFAIL 0x00e8 +#define USBMSC_TRACEERR_WRALLOCREQ 0x00e9 +#define USBMSC_TRACEERR_WRCOMPLETEINVALIDARGS 0x00ea +#define USBMSC_TRACEERR_WRITE10FLAGS 0x00eb +#define USBMSC_TRACEERR_WRITE10LBARANGE 0x00ec +#define USBMSC_TRACEERR_WRITE10MEDIANOTPRESENT 0x00ed +#define USBMSC_TRACEERR_WRITE10READONLY 0x00ee +#define USBMSC_TRACEERR_WRITE12FLAGS 0x00ef +#define USBMSC_TRACEERR_WRITE12LBARANGE 0x00f0 +#define USBMSC_TRACEERR_WRITE12MEDIANOTPRESENT 0x00f1 +#define USBMSC_TRACEERR_WRITE12READONLY 0x00f2 +#define USBMSC_TRACEERR_WRITE6LBARANGE 0x00f3 +#define USBMSC_TRACEERR_WRITE6MEDIANOTPRESENT 0x00f4 +#define USBMSC_TRACEERR_WRITE6READONLY 0x00f5 +#define USBMSC_TRACEERR_WRSHUTDOWN 0x00f6 +#define USBMSC_TRACEERR_WRUNEXPECTED 0x00f7 +#define USBMSC_TRACEERR_UNSUPPORTEDTYPE 0x00f8 /**************************************************************************** * Public Types