diff options
author | patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3> | 2013-02-02 00:29:55 +0000 |
---|---|---|
committer | patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3> | 2013-02-02 00:29:55 +0000 |
commit | d456ef7910458353f84d1270f5a9061441a082bc (patch) | |
tree | cebe6b657efd2e135a9ddcc10554df00e7cfebef | |
parent | 7e161b4a2da17da233a1869ce6f44582ae7692e4 (diff) | |
download | px4-nuttx-d456ef7910458353f84d1270f5a9061441a082bc.tar.gz px4-nuttx-d456ef7910458353f84d1270f5a9061441a082bc.tar.bz2 px4-nuttx-d456ef7910458353f84d1270f5a9061441a082bc.zip |
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
-rw-r--r-- | nuttx/ChangeLog | 3 | ||||
-rw-r--r-- | nuttx/drivers/serial/serial.c | 104 |
2 files changed, 78 insertions, 29 deletions
diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index e2e7ae29b..5bdd1aad8 100644 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -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. diff --git a/nuttx/drivers/serial/serial.c b/nuttx/drivers/serial/serial.c index cc1c697c0..68f0f43a2 100644 --- a/nuttx/drivers/serial/serial.c +++ b/nuttx/drivers/serial/serial.c @@ -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. */ |