From 64a92df4554663a47c3d63348cac9c8d1ad13b1c Mon Sep 17 00:00:00 2001 From: Hasnain Virk Date: Tue, 13 Jun 2017 15:25:48 +0300 Subject: [PATCH] Avoid lock collision b/w SerialBase & UARTSerial Fixes issue #4537. SerialBase and UARTSerial happened to have same names for the mutex locking that confused the system of holding a mutex in interrupt context. UARTSerial has to change so as to provide empty implementations for lock() and unlock() becuase it uses SerialBase from interrupt context only or from its own critical section, so no extra locks required. Private locks for UARTSerial itself are renamed to api_lock() and api_unlock(). --- drivers/UARTSerial.cpp | 48 ++++++++++++++++++++++++++---------------- drivers/UARTSerial.h | 27 +++++++++++++++--------- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/drivers/UARTSerial.cpp b/drivers/UARTSerial.cpp index 19d8174dbcc..7cb15fa6897 100644 --- a/drivers/UARTSerial.cpp +++ b/drivers/UARTSerial.cpp @@ -14,7 +14,7 @@ * limitations under the License. */ -#if DEVICE_SERIAL +#if (DEVICE_SERIAL && DEVICE_INTERRUPTIN) #include #include "UARTSerial.h" @@ -80,16 +80,16 @@ off_t UARTSerial::seek(off_t offset, int whence) int UARTSerial::sync() { - lock(); + api_lock(); while (!_txbuf.empty()) { - unlock(); + api_unlock(); // Doing better than wait would require TxIRQ to also do wake() when becoming empty. Worth it? wait_ms(1); - lock(); + api_lock(); } - unlock(); + api_unlock(); return 0; } @@ -111,16 +111,16 @@ ssize_t UARTSerial::write(const void* buffer, size_t length) size_t data_written = 0; const char *buf_ptr = static_cast(buffer); - lock(); + api_lock(); while (_txbuf.full()) { if (!_blocking) { - unlock(); + api_unlock(); return -EAGAIN; } - unlock(); + api_unlock(); wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ? - lock(); + api_lock(); } while (data_written < length && !_txbuf.full()) { @@ -138,7 +138,7 @@ ssize_t UARTSerial::write(const void* buffer, size_t length) } core_util_critical_section_exit(); - unlock(); + api_unlock(); return data_written; } @@ -149,16 +149,16 @@ ssize_t UARTSerial::read(void* buffer, size_t length) char *ptr = static_cast(buffer); - lock(); + api_lock(); while (_rxbuf.empty()) { if (!_blocking) { - unlock(); + api_unlock(); return -EAGAIN; } - unlock(); + api_unlock(); wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ? - lock(); + api_lock(); } while (data_read < length && !_rxbuf.empty()) { @@ -166,7 +166,7 @@ ssize_t UARTSerial::read(void* buffer, size_t length) data_read++; } - unlock(); + api_unlock(); return data_read; } @@ -205,12 +205,24 @@ short UARTSerial::poll(short events) const { return revents; } -void UARTSerial::lock(void) +void UARTSerial::lock() +{ + // This is the override for SerialBase. + // No lock required as we only use SerialBase from interrupt or from + // inside our own critical section. +} + +void UARTSerial::unlock() +{ + // This is the override for SerialBase. +} + +void UARTSerial::api_lock(void) { _mutex.lock(); } -void UARTSerial::unlock(void) +void UARTSerial::api_unlock(void) { _mutex.unlock(); } @@ -262,4 +274,4 @@ void UARTSerial::tx_irq(void) } //namespace mbed -#endif //DEVICE_SERIAL +#endif //(DEVICE_SERIAL && DEVICE_INTERRUPTIN) diff --git a/drivers/UARTSerial.h b/drivers/UARTSerial.h index fda877d8960..dc66be40705 100644 --- a/drivers/UARTSerial.h +++ b/drivers/UARTSerial.h @@ -19,7 +19,7 @@ #include "platform/platform.h" -#if DEVICE_SERIAL +#if (DEVICE_SERIAL && DEVICE_INTERRUPTIN) || defined(DOXYGEN_ONLY) #include "FileHandle.h" #include "SerialBase.h" @@ -58,7 +58,7 @@ class UARTSerial : private SerialBase, public FileHandle { /** Write the contents of a buffer to a file * * @param buffer The buffer to write from - * @param size The number of bytes to write + * @param length The number of bytes to write * @return The number of bytes written, negative error on failure */ virtual ssize_t write(const void* buffer, size_t length); @@ -72,17 +72,11 @@ class UARTSerial : private SerialBase, public FileHandle { * * If any data is available, call returns immediately * * @param buffer The buffer to read in to - * @param size The number of bytes to read + * @param length The number of bytes to read * @return The number of bytes read, 0 at end of file, negative error on failure */ virtual ssize_t read(void* buffer, size_t length); - /** Acquire mutex */ - virtual void lock(void); - - /** Release mutex */ - virtual void unlock(void); - /** Close a file * * @return 0 on success, negative error code on failure @@ -159,6 +153,18 @@ class UARTSerial : private SerialBase, public FileHandle { private: + /** SerialBase lock override */ + virtual void lock(void); + + /** SerialBase unlock override */ + virtual void unlock(void); + + /** Acquire mutex */ + virtual void api_lock(void); + + /** Release mutex */ + virtual void api_unlock(void); + /** Software serial buffers * By default buffer size is 256 for TX and 256 for RX. Configurable through mbed_app.json */ @@ -191,8 +197,9 @@ class UARTSerial : private SerialBase, public FileHandle { void wake(void); void dcd_irq(void); + }; } //namespace mbed -#endif //DEVICE_SERIAL +#endif //(DEVICE_SERIAL && DEVICE_INTERRUPTIN) || defined(DOXYGEN_ONLY) #endif //MBED_UARTSERIAL_H