Skip to content

Commit 831aaef

Browse files
committed
Fix lock ordering in dist_nifs.c to prevent SMP deadlocks
https://ampcode.com/threads/T-019ba265-8657-732e-a85b-9871a0214ae2 Fix lock ordering inversions in `dist_nifs.c` that can cause deadlocks under SMP when distribution connections are used concurrently with process termination. ## Problem Similar to the recently fixed monitor/demonitor deadlock, `dist_nifs.c` calls `enif_demonitor_process` while holding the `remote_monitors` lock. Since `enif_demonitor_process` acquires `monitors` → `processes_table`, this creates an ABBA deadlock risk: | Code Path | Lock Order | |-----------|------------| | `dist_connection_dtor` | `remote_monitors` → `monitors` → `processes_table` | | `OPERATION_DEMONITOR_P` | `remote_monitors` → `monitors` → `processes_table` | | `context_destroy` (firing resource monitors) | `processes_table` → `monitors` | When a distribution connection is being destroyed while a monitored process is terminating, these paths can deadlock. ## Solution Release the `remote_monitors` lock before calling `enif_demonitor_process`: 1. **`dist_connection_dtor`**: Move all monitors to a local list, unlock, then iterate and demonitor 2. **`OPERATION_DEMONITOR_P`**: Remove the found monitor from the list, unlock, then demonitor This ensures consistent lock ordering: `processes_table` → `monitors` is always acquired before any synclist locks that might call back into the monitor/demonitor APIs. Signed-off-by: Peter M <petermm@gmail.com>
1 parent 64badd3 commit 831aaef

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

src/libAtomVM/dist_nifs.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,19 @@ static void dist_connection_dtor(ErlNifEnv *caller_env, void *obj)
117117
struct ListHead *remote_monitors = synclist_wrlock(&conn_obj->remote_monitors);
118118
struct ListHead *item;
119119
struct ListHead *tmp;
120+
struct ListHead to_demonitor;
121+
list_init(&to_demonitor);
120122
MUTABLE_LIST_FOR_EACH (item, tmp, remote_monitors) {
123+
list_remove(item);
124+
list_append(&to_demonitor, item);
125+
}
126+
synclist_unlock(&conn_obj->remote_monitors);
127+
MUTABLE_LIST_FOR_EACH (item, tmp, &to_demonitor) {
121128
struct RemoteMonitor *remote_monitor = GET_LIST_ENTRY(item, struct RemoteMonitor, head);
122129
enif_demonitor_process(caller_env, conn_obj, &remote_monitor->process_monitor);
123130
list_remove(item);
124131
free(item);
125132
}
126-
synclist_unlock(&conn_obj->remote_monitors);
127133
synclist_destroy(&conn_obj->remote_monitors);
128134
struct ListHead *pending_packets = synclist_wrlock(&conn_obj->pending_packets);
129135
MUTABLE_LIST_FOR_EACH (item, tmp, pending_packets) {
@@ -531,20 +537,24 @@ static term nif_erlang_dist_ctrl_put_data(Context *ctx, int argc, term argv[])
531537
term monitor_ref = term_get_tuple_element(control, 3);
532538
uint8_t ref_len = term_get_external_reference_len(monitor_ref);
533539
const uint32_t *ref_words = term_get_external_reference_words(monitor_ref);
540+
struct RemoteMonitor *found_monitor = NULL;
534541
struct ListHead *remote_monitors = synclist_wrlock(&conn_obj->remote_monitors);
535542
struct ListHead *item;
536543
LIST_FOR_EACH (item, remote_monitors) {
537544
struct RemoteMonitor *remote_monitor = GET_LIST_ENTRY(item, struct RemoteMonitor, head);
538545
if (remote_monitor->target_proc == target_proc
539546
&& remote_monitor->ref_len == ref_len
540547
&& memcmp(remote_monitor->ref_words, ref_words, ref_len * sizeof(uint32_t)) == 0) {
541-
enif_demonitor_process(erl_nif_env_from_context(ctx), conn_obj, &remote_monitor->process_monitor);
542548
list_remove(item);
543-
free(remote_monitor);
549+
found_monitor = remote_monitor;
544550
break;
545551
}
546552
}
547553
synclist_unlock(&conn_obj->remote_monitors);
554+
if (found_monitor) {
555+
enif_demonitor_process(erl_nif_env_from_context(ctx), conn_obj, &found_monitor->process_monitor);
556+
free(found_monitor);
557+
}
548558
break;
549559
}
550560
case OPERATION_SEND_SENDER: {

0 commit comments

Comments
 (0)