Skip to content

Commit 0109ff7

Browse files
committed
Deadlock distnifs
https://ampcode.com/threads/T-019beca5-7177-747e-9114-1022dc7a930a Signed-off-by: Peter M <petermm@gmail.com>
1 parent fd96d95 commit 0109ff7

File tree

1 file changed

+35
-5
lines changed

1 file changed

+35
-5
lines changed

src/libAtomVM/dist_nifs.c

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,28 +111,58 @@ struct DistConnection
111111
static void dist_connection_dtor(ErlNifEnv *caller_env, void *obj)
112112
{
113113
struct DistConnection *conn_obj = (struct DistConnection *) obj;
114+
115+
// IMPORTANT: Remove from global list FIRST to maintain lock ordering.
116+
// Lock order must always be: dist_connections -> remote_monitors -> pending_packets
117+
// This prevents ABBA deadlock with code paths that hold dist_connections
118+
// while acquiring pending_packets (e.g., dist_send_message, dist_send_link).
119+
synclist_remove(&caller_env->global->dist_connections, &conn_obj->head);
120+
114121
if (conn_obj->connection_process_id != INVALID_PROCESS_ID) {
115122
enif_demonitor_process(caller_env, conn_obj, &conn_obj->connection_process_monitor);
116123
}
124+
125+
// Collect monitors to demonitor outside the lock to avoid holding lock
126+
// across potentially blocking enif_demonitor_process calls
127+
struct ListHead monitors_to_free;
128+
list_init(&monitors_to_free);
129+
117130
struct ListHead *remote_monitors = synclist_wrlock(&conn_obj->remote_monitors);
118131
struct ListHead *item;
119132
struct ListHead *tmp;
120133
MUTABLE_LIST_FOR_EACH (item, tmp, remote_monitors) {
121-
struct RemoteMonitor *remote_monitor = GET_LIST_ENTRY(item, struct RemoteMonitor, head);
122-
enif_demonitor_process(caller_env, conn_obj, &remote_monitor->process_monitor);
123134
list_remove(item);
124-
free(item);
135+
list_append(&monitors_to_free, item);
125136
}
126137
synclist_unlock(&conn_obj->remote_monitors);
127138
synclist_destroy(&conn_obj->remote_monitors);
139+
140+
// Now demonitor and free outside the lock
141+
MUTABLE_LIST_FOR_EACH (item, tmp, &monitors_to_free) {
142+
struct RemoteMonitor *remote_monitor = GET_LIST_ENTRY(item, struct RemoteMonitor, head);
143+
enif_demonitor_process(caller_env, conn_obj, &remote_monitor->process_monitor);
144+
list_remove(item);
145+
free(remote_monitor);
146+
}
147+
148+
// Collect packets to free outside the lock
149+
struct ListHead packets_to_free;
150+
list_init(&packets_to_free);
151+
128152
struct ListHead *pending_packets = synclist_wrlock(&conn_obj->pending_packets);
129153
MUTABLE_LIST_FOR_EACH (item, tmp, pending_packets) {
130154
list_remove(item);
131-
free(item);
155+
list_append(&packets_to_free, item);
132156
}
133157
synclist_unlock(&conn_obj->pending_packets);
134158
synclist_destroy(&conn_obj->pending_packets);
135-
synclist_remove(&caller_env->global->dist_connections, &conn_obj->head);
159+
160+
// Free packets outside the lock
161+
MUTABLE_LIST_FOR_EACH (item, tmp, &packets_to_free) {
162+
struct DistributionPacket *packet = GET_LIST_ENTRY(item, struct DistributionPacket, head);
163+
list_remove(item);
164+
free(packet);
165+
}
136166
}
137167

138168
static void dist_enqueue_message(term control_message, term payload, struct DistConnection *connection, GlobalContext *global)

0 commit comments

Comments
 (0)