drivers/serial.c: Fix some race conditions. Some bad things code happen if we lost a USB connection at certain times.

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@5596 42af7a65-404d-4744-a932-0658087f49c3
This commit is contained in:
patacongo 2013-02-02 00:29:55 +00:00
parent 9a05ccdbbc
commit 11aa231cf0
2 changed files with 78 additions and 29 deletions

View File

@ -4097,3 +4097,6 @@
well. From Petteri Aimonen.
6.26 2013-xx-xx Gregory Nutt <gnutt@nuttx.org>
* drivers/serial/serial.c: Correct some race conditions when checking
for disconnection of a removable serial device.

View File

@ -211,27 +211,40 @@ static int uart_putxmitchar(FAR uart_dev_t *dev, int ch)
*/
flags = irqsave();
dev->xmitwaiting = true;
#ifdef CONFIG_SERIAL_REMOVABLE
/* Check if the removable device is no longer connected while we
* have interrupts off. We do not want the transition to occur
* as a race condition before we begin the wait.
*/
if (dev->disconnected)
{
irqrestore(flags);
return -ENOTCONN;
}
#endif
/* Wait for some characters to be sent from the buffer with the TX
* interrupt enabled. When the TX interrupt is enabled, uart_xmitchars
* should execute and remove some of the data from the TX buffer.
*/
dev->xmitwaiting = true;
uart_enabletxint(dev);
ret = uart_takesem(&dev->xmitsem, true);
uart_disabletxint(dev);
irqrestore(flags);
#ifdef CONFIG_SERIAL_REMOVABLE
/* Check if the removable device is no longer connected */
/* Check if the removable device was disconnected while we were
* waiting.
*/
if (dev->disconnected)
{
return -ENOTCONN;
}
#endif
/* Check if we were awakened by signal. */
if (ret < 0)
@ -342,7 +355,9 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer, size_t
#ifdef CONFIG_SERIAL_REMOVABLE
/* If the removable device is no longer connected, refuse to write to the
* device.
* device. This check occurs after taking the xmit.sem because the
* disconnection event might have occurred while we were waiting for
* access to the transmit buffers.
*/
if (dev->disconnected)
@ -377,8 +392,12 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer, size_t
ret = uart_putxmitchar(dev, ch);
}
/* Were we awakened by a signal? That should be the only condition that
* uart_putxmitchar() should return an error.
/* uart_putxmitchar() might return an error under one of two
* conditions: (1) The wait for buffer space might have been
* interrupted by a signal (ret should be -EINTR), or (2) if
* CONFIG_SERIAL_REMOVABLE is defined, then uart_putxmitchar()
* might also return if the serial device was disconnected
* (wtih -ENOTCONN).
*/
if (ret < 0)
@ -402,7 +421,7 @@ static ssize_t uart_write(FAR struct file *filep, FAR const char *buffer, size_t
* set the errno value appropriately).
*/
nread = -EINTR;
nread = ret;
}
break;
@ -444,16 +463,6 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen
return ret;
}
#ifdef CONFIG_SERIAL_REMOVABLE
/* If the removable device is no longer connected, refuse to read from the device */
if (dev->disconnected)
{
uart_givesem(&dev->recv.sem);
return -ENOTCONN;
}
#endif
/* Loop while we still have data to copy to the receive buffer.
* we add data to the head of the buffer; uart_xmitchars takes the
* data from the end of the buffer.
@ -461,6 +470,22 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen
while (recvd < buflen)
{
#ifdef CONFIG_SERIAL_REMOVABLE
/* If the removable device is no longer connected, refuse to read any
* further from the device.
*/
if (dev->disconnected)
{
if (recvd == 0)
{
recvd = -ENOTCONN;
}
break;
}
#endif
/* Check if there is more data to return in the circular buffer.
* NOTE: Rx interrupt handling logic may aynchronously increment
* the head index but must not modify the tail index. The tail
@ -509,6 +534,7 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen
{
recvd = -EAGAIN;
}
break;
}
#else
@ -560,19 +586,34 @@ static ssize_t uart_read(FAR struct file *filep, FAR char *buffer, size_t buflen
*/
flags = irqsave();
dev->recvwaiting = true;
uart_enablerxint(dev);
#ifdef CONFIG_SERIAL_REMOVABLE
/* Check if the removable device is no longer connected while
* we have interrupts off. We do not want the transition to
* occur as a race condition before we begin the wait.
*/
if (dev->disconnected)
{
uart_enablerxint(dev);
irqrestore(flags);
ret = -ENOTCONN;
break;
}
#endif
/* Now wait with the Rx interrupt re-enabled. NuttX will
* automatically re-enable global interrupts when this thread
* goes to sleep.
*/
dev->recvwaiting = true;
uart_enablerxint(dev);
ret = uart_takesem(&dev->recvsem, true);
irqrestore(flags);
/* Was a signal received while waiting for data to be
* received? Was a removable device disconnected?
* received? Was a removable device disconnected while
* we were waiting?
*/
#ifdef CONFIG_SERIAL_REMOVABLE
@ -840,6 +881,7 @@ static int uart_close(FAR struct file *filep)
{
uart_shutdown(dev); /* Disable the UART */
}
irqrestore(flags);
uart_givesem(&dev->closesem);
@ -861,15 +903,6 @@ static int uart_open(FAR struct file *filep)
uint8_t tmp;
int ret;
#ifdef CONFIG_SERIAL_REMOVABLE
/* If the removable device is no longer connected, refuse to open the device */
if (dev->disconnected)
{
return -ENOTCONN;
}
#endif
/* If the port is the middle of closing, wait until the close is finished.
* If a signal is received while we are waiting, then return EINTR.
*/
@ -882,6 +915,19 @@ static int uart_open(FAR struct file *filep)
return ret;
}
#ifdef CONFIG_SERIAL_REMOVABLE
/* If the removable device is no longer connected, refuse to open the
* device. We check this after obtaining the close semaphore because
* we might have been waiting when the device was disconnected.
*/
if (dev->disconnected)
{
ret = -ENOTCONN;
goto errout_with_sem;
}
#endif
/* Start up serial port */
/* Increment the count of references to the device. */