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

This commit is contained in:
Gregory Nutt 2013-11-01 12:09:25 -06:00
parent af4c5246b0
commit 7bbc46f721
4 changed files with 73 additions and 28 deletions

View File

@ -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).

View File

@ -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;

View File

@ -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 wouldnt
* 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;
}

View File

@ -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