Skip to content
This repository was archived by the owner on Aug 5, 2022. It is now read-only.

Commit 88e6cf8

Browse files
authored
[uart] Protect ring buffer access in ISRs w/ irq_lock not mutex (#1569)
It's not safe to be locking / unlocking mutexes from an ISR but we were doing that because zjs_signal_callback is intended to be called by ISRs. This was causing a hang in UART module after more careful locking was added recently. Fixes #1541 Signed-off-by: Geoff Gustafson <[email protected]>
1 parent cbe7e8f commit 88e6cf8

File tree

1 file changed

+44
-17
lines changed

1 file changed

+44
-17
lines changed

src/zjs_callbacks.c

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ SYS_RING_BUF_DECLARE_POW2(ring_buffer, 5);
9595
static u8_t ring_buf_initialized = 1;
9696

9797
#ifdef ZJS_LINUX_BUILD
98+
#define k_is_preempt_thread() 0
99+
#define irq_lock() 0
100+
#define irq_unlock(key) do {} while (0);
98101
#define RB_LOCK() do {} while (0)
99102
#define RB_UNLOCK() do {} while (0)
100103
#define CB_LOCK() do {} while (0)
@@ -103,26 +106,42 @@ static u8_t ring_buf_initialized = 1;
103106
// mutex to ensure only one thread can access ring buffer at a time
104107
static struct k_mutex ring_mutex;
105108

106-
#define RB_LOCK() \
107-
LPRINT("Ringbuf lock..."); \
108-
k_mutex_lock(&ring_mutex, K_FOREVER); \
109-
LPRINT("Ringbuf locked.")
110-
#define RB_UNLOCK() \
109+
#define RB_LOCK() \
110+
{ \
111+
LPRINT("Ringbuf lock..."); \
112+
if (k_mutex_lock(&ring_mutex, K_FOREVER)) { \
113+
ERR_PRINT("Ringbuf lock error.\n"); \
114+
} else { \
115+
LPRINT("Ringbuf locked."); \
116+
} \
117+
}
118+
119+
#define RB_UNLOCK() \
120+
{ \
111121
LPRINT("Ringbuf unlock..."); \
112122
k_mutex_unlock(&ring_mutex); \
113-
LPRINT("Ringbuf unlocked.")
123+
LPRINT("Ringbuf unlocked."); \
124+
}
114125

115126
// mutex to ensure only one thread can access cb_map at a time
116127
static struct k_mutex cb_mutex;
117128

118-
#define CB_LOCK() \
119-
LPRINT("Callbacks lock..."); \
120-
k_mutex_lock(&cb_mutex, K_FOREVER); \
121-
LPRINT("Callbacks locked.")
129+
#define CB_LOCK() \
130+
{ \
131+
LPRINT("Callbacks lock..."); \
132+
if (k_mutex_lock(&cb_mutex, K_FOREVER)) { \
133+
ERR_PRINT("Callback lock error.\n"); \
134+
} else { \
135+
LPRINT("Callbacks locked."); \
136+
} \
137+
}
138+
122139
#define CB_UNLOCK() \
140+
{ \
123141
LPRINT("Callbacks unlock..."); \
124142
k_mutex_unlock(&cb_mutex); \
125-
LPRINT("Callbacks unlocked.")
143+
LPRINT("Callbacks unlocked."); \
144+
}
126145
#endif // ZJS_LINUX_BUILD
127146

128147
static zjs_callback_id cb_limit = INITIAL_CALLBACK_SIZE;
@@ -371,15 +390,17 @@ void signal_callback_priv(zjs_callback_id id,
371390
DBG_PRINT("pushing item to ring buffer. id=%d, args=%p, size=%u\n", id,
372391
args, size);
373392
#endif
374-
CB_LOCK();
393+
int in_thread = k_is_preempt_thread(); // versus ISR or co-op thread
394+
int key = 0;
395+
if (in_thread) CB_LOCK();
375396
if (id < 0 || id >= cb_size || !cb_map[id]) {
376397
DBG_PRINT("callback ID %u does not exist\n", id);
377-
CB_UNLOCK();
398+
if (in_thread) CB_UNLOCK();
378399
return;
379400
}
380401
if (GET_CB_REMOVED(cb_map[id]->flags)) {
381402
DBG_PRINT("callback already removed\n");
382-
CB_UNLOCK();
403+
if (in_thread) CB_UNLOCK();
383404
return;
384405
}
385406
if (GET_TYPE(cb_map[id]->flags) == CALLBACK_TYPE_JS) {
@@ -393,7 +414,10 @@ void signal_callback_priv(zjs_callback_id id,
393414
#ifdef INSTRUMENT_CALLBACKS
394415
set_info_string(cb_map[id]->caller, file, func);
395416
#endif
396-
RB_LOCK();
417+
if (in_thread) {
418+
RB_LOCK();
419+
key = irq_lock();
420+
}
397421
int ret = zjs_port_ring_buf_put(&ring_buffer,
398422
(u16_t)id,
399423
0, // we use value for CB_FLUSH_ONE/ALL
@@ -404,7 +428,10 @@ void signal_callback_priv(zjs_callback_id id,
404428
// rather than locking everything, as we are only trying to prevent a
405429
// callback
406430
// from being edited and called at the same time.
407-
RB_UNLOCK();
431+
if (in_thread) {
432+
irq_unlock(key);
433+
RB_UNLOCK();
434+
}
408435
#ifndef ZJS_LINUX_BUILD
409436
zjs_loop_unblock();
410437
#endif
@@ -421,7 +448,7 @@ void signal_callback_priv(zjs_callback_id id,
421448
zjs_ringbuf_error_count++;
422449
zjs_ringbuf_last_error = ret;
423450
}
424-
CB_UNLOCK();
451+
if (in_thread) CB_UNLOCK();
425452
}
426453

427454
zjs_callback_id zjs_add_c_callback(void *handle, zjs_c_callback_func callback)

0 commit comments

Comments
 (0)