Skip to content

Commit 64a92df

Browse files
author
Hasnain Virk
committed
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().
1 parent 35999be commit 64a92df

File tree

2 files changed

+47
-28
lines changed

2 files changed

+47
-28
lines changed

drivers/UARTSerial.cpp

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
#if DEVICE_SERIAL
17+
#if (DEVICE_SERIAL && DEVICE_INTERRUPTIN)
1818

1919
#include <errno.h>
2020
#include "UARTSerial.h"
@@ -80,16 +80,16 @@ off_t UARTSerial::seek(off_t offset, int whence)
8080

8181
int UARTSerial::sync()
8282
{
83-
lock();
83+
api_lock();
8484

8585
while (!_txbuf.empty()) {
86-
unlock();
86+
api_unlock();
8787
// Doing better than wait would require TxIRQ to also do wake() when becoming empty. Worth it?
8888
wait_ms(1);
89-
lock();
89+
api_lock();
9090
}
9191

92-
unlock();
92+
api_unlock();
9393

9494
return 0;
9595
}
@@ -111,16 +111,16 @@ ssize_t UARTSerial::write(const void* buffer, size_t length)
111111
size_t data_written = 0;
112112
const char *buf_ptr = static_cast<const char *>(buffer);
113113

114-
lock();
114+
api_lock();
115115

116116
while (_txbuf.full()) {
117117
if (!_blocking) {
118-
unlock();
118+
api_unlock();
119119
return -EAGAIN;
120120
}
121-
unlock();
121+
api_unlock();
122122
wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ?
123-
lock();
123+
api_lock();
124124
}
125125

126126
while (data_written < length && !_txbuf.full()) {
@@ -138,7 +138,7 @@ ssize_t UARTSerial::write(const void* buffer, size_t length)
138138
}
139139
core_util_critical_section_exit();
140140

141-
unlock();
141+
api_unlock();
142142

143143
return data_written;
144144
}
@@ -149,24 +149,24 @@ ssize_t UARTSerial::read(void* buffer, size_t length)
149149

150150
char *ptr = static_cast<char *>(buffer);
151151

152-
lock();
152+
api_lock();
153153

154154
while (_rxbuf.empty()) {
155155
if (!_blocking) {
156-
unlock();
156+
api_unlock();
157157
return -EAGAIN;
158158
}
159-
unlock();
159+
api_unlock();
160160
wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ?
161-
lock();
161+
api_lock();
162162
}
163163

164164
while (data_read < length && !_rxbuf.empty()) {
165165
_rxbuf.pop(*ptr++);
166166
data_read++;
167167
}
168168

169-
unlock();
169+
api_unlock();
170170

171171
return data_read;
172172
}
@@ -205,12 +205,24 @@ short UARTSerial::poll(short events) const {
205205
return revents;
206206
}
207207

208-
void UARTSerial::lock(void)
208+
void UARTSerial::lock()
209+
{
210+
// This is the override for SerialBase.
211+
// No lock required as we only use SerialBase from interrupt or from
212+
// inside our own critical section.
213+
}
214+
215+
void UARTSerial::unlock()
216+
{
217+
// This is the override for SerialBase.
218+
}
219+
220+
void UARTSerial::api_lock(void)
209221
{
210222
_mutex.lock();
211223
}
212224

213-
void UARTSerial::unlock(void)
225+
void UARTSerial::api_unlock(void)
214226
{
215227
_mutex.unlock();
216228
}
@@ -262,4 +274,4 @@ void UARTSerial::tx_irq(void)
262274

263275
} //namespace mbed
264276

265-
#endif //DEVICE_SERIAL
277+
#endif //(DEVICE_SERIAL && DEVICE_INTERRUPTIN)

drivers/UARTSerial.h

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
#include "platform/platform.h"
2121

22-
#if DEVICE_SERIAL
22+
#if (DEVICE_SERIAL && DEVICE_INTERRUPTIN) || defined(DOXYGEN_ONLY)
2323

2424
#include "FileHandle.h"
2525
#include "SerialBase.h"
@@ -58,7 +58,7 @@ class UARTSerial : private SerialBase, public FileHandle {
5858
/** Write the contents of a buffer to a file
5959
*
6060
* @param buffer The buffer to write from
61-
* @param size The number of bytes to write
61+
* @param length The number of bytes to write
6262
* @return The number of bytes written, negative error on failure
6363
*/
6464
virtual ssize_t write(const void* buffer, size_t length);
@@ -72,17 +72,11 @@ class UARTSerial : private SerialBase, public FileHandle {
7272
* * If any data is available, call returns immediately
7373
*
7474
* @param buffer The buffer to read in to
75-
* @param size The number of bytes to read
75+
* @param length The number of bytes to read
7676
* @return The number of bytes read, 0 at end of file, negative error on failure
7777
*/
7878
virtual ssize_t read(void* buffer, size_t length);
7979

80-
/** Acquire mutex */
81-
virtual void lock(void);
82-
83-
/** Release mutex */
84-
virtual void unlock(void);
85-
8680
/** Close a file
8781
*
8882
* @return 0 on success, negative error code on failure
@@ -159,6 +153,18 @@ class UARTSerial : private SerialBase, public FileHandle {
159153

160154
private:
161155

156+
/** SerialBase lock override */
157+
virtual void lock(void);
158+
159+
/** SerialBase unlock override */
160+
virtual void unlock(void);
161+
162+
/** Acquire mutex */
163+
virtual void api_lock(void);
164+
165+
/** Release mutex */
166+
virtual void api_unlock(void);
167+
162168
/** Software serial buffers
163169
* By default buffer size is 256 for TX and 256 for RX. Configurable through mbed_app.json
164170
*/
@@ -191,8 +197,9 @@ class UARTSerial : private SerialBase, public FileHandle {
191197
void wake(void);
192198

193199
void dcd_irq(void);
200+
194201
};
195202
} //namespace mbed
196203

197-
#endif //DEVICE_SERIAL
204+
#endif //(DEVICE_SERIAL && DEVICE_INTERRUPTIN) || defined(DOXYGEN_ONLY)
198205
#endif //MBED_UARTSERIAL_H

0 commit comments

Comments
 (0)