From 49504906b4a16b69e52beb09e92f68c6ccd51753 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 11 Sep 2018 16:56:29 +0200 Subject: erts: Fix "Prevent inconsistent node lists" fix done in a31216200bdee2c04b3fb3ae5e26607674715c8a that could cause a new pending connection to be incorrectly aborted. --- erts/emulator/beam/dist.c | 23 +++++----- erts/emulator/beam/dist.h | 2 +- erts/emulator/beam/erl_node_tables.c | 83 +++++++++++++++++++----------------- 3 files changed, 57 insertions(+), 51 deletions(-) diff --git a/erts/emulator/beam/dist.c b/erts/emulator/beam/dist.c index 16c4d689a5..1e822d5c7b 100644 --- a/erts/emulator/beam/dist.c +++ b/erts/emulator/beam/dist.c @@ -3563,20 +3563,18 @@ BIF_RETTYPE erts_internal_new_connection_1(BIF_ALIST_1) BIF_RET(TUPLE2(hp, make_small(conn_id), dhandle)); } -static Sint abort_connection(DistEntry* dep, Uint32 conn_id) +Sint erts_abort_connection_rwunlock(DistEntry* dep) { - erts_de_rwlock(dep); + Sint reds = 0; + ERTS_LC_ASSERT(erts_lc_is_de_rwlocked(dep)); - if (dep->connection_id != conn_id) - ; - else if (dep->state == ERTS_DE_STATE_CONNECTED) { + if (dep->state == ERTS_DE_STATE_CONNECTED) { kill_connection(dep); } else if (dep->state == ERTS_DE_STATE_PENDING) { ErtsAtomCache *cache; ErtsDistOutputBuf *obuf; ErtsProcList *resume_procs; - Sint reds = 0; ErtsMonLnkDist *mld; ASSERT(is_nil(dep->cid)); @@ -3621,17 +3619,18 @@ static Sint abort_connection(DistEntry* dep, Uint32 conn_id) erts_de_rwlock(dep); ASSERT(dep->state == ERTS_DE_STATE_EXITING); dep->state = ERTS_DE_STATE_IDLE; - erts_de_rwunlock(dep); - return reds; } erts_de_rwunlock(dep); - return 0; + return reds; } -Sint -erts_abort_connection(DistEntry *dep, Uint32 conn_id) +static Sint abort_connection(DistEntry *dep, Uint32 conn_id) { - return abort_connection(dep, conn_id); + erts_de_rwlock(dep); + if (dep->connection_id == conn_id) + return erts_abort_connection_rwunlock(dep); + erts_de_rwunlock(dep); + return 0; } BIF_RETTYPE erts_internal_abort_connection_2(BIF_ALIST_2) diff --git a/erts/emulator/beam/dist.h b/erts/emulator/beam/dist.h index 30b4b35c20..75cb865390 100644 --- a/erts/emulator/beam/dist.h +++ b/erts/emulator/beam/dist.h @@ -399,7 +399,7 @@ extern void erts_kill_dist_connection(DistEntry *dep, Uint32); extern Uint erts_dist_cache_size(void); -extern Sint erts_abort_connection(DistEntry *dep, Uint32 conn_id); +extern Sint erts_abort_connection_rwunlock(DistEntry *dep); #endif diff --git a/erts/emulator/beam/erl_node_tables.c b/erts/emulator/beam/erl_node_tables.c index 9b34af1480..f4a36d124a 100644 --- a/erts/emulator/beam/erl_node_tables.c +++ b/erts/emulator/beam/erl_node_tables.c @@ -60,6 +60,10 @@ static int references_atoms_need_init = 1; static ErtsMonotonicTime orig_node_tab_delete_delay; static ErtsMonotonicTime node_tab_delete_delay; + +static void report_gc_active_dist_entry(Eterm sysname, enum dist_entry_state); + + /* -- The distribution table ---------------------------------------------- */ #define ErtsBin2DistEntry(B) \ @@ -412,44 +416,6 @@ static void schedule_delete_dist_entry(DistEntry* dep) static void start_timer_delete_dist_entry(void *vdep) { - DistEntry *dep = vdep; - Eterm sysname; - enum dist_entry_state state; - Uint32 connection_id; - - erts_de_rlock(dep); - state = dep->state; - connection_id = dep->connection_id; - sysname = dep->sysname; - erts_de_runlock(dep); - - if (state != ERTS_DE_STATE_IDLE) { - char *state_str; - erts_dsprintf_buf_t *dsbuf = erts_create_logger_dsbuf(); - switch (state) { - case ERTS_DE_STATE_CONNECTED: - state_str = "connected"; - break; - case ERTS_DE_STATE_PENDING: - state_str = "pending connect"; - break; - case ERTS_DE_STATE_EXITING: - state_str = "exiting"; - break; - case ERTS_DE_STATE_IDLE: - state_str = "idle"; - break; - default: - state_str = "unknown"; - break; - } - erts_dsprintf(dsbuf, "Garbage collecting distribution " - "entry for node %T in state: %s", - sysname, state_str); - erts_send_error_to_logger_nogl(dsbuf); - erts_abort_connection(dep, connection_id); - } - if (node_tab_delete_delay == 0) { prepare_try_delete_dist_entry(vdep); } @@ -489,6 +455,19 @@ static void try_delete_dist_entry(DistEntry* dep) { erts_aint_t refc; + erts_de_rwlock(dep); + if (dep->state != ERTS_DE_STATE_IDLE && de_refc_read(dep,0) == 0) { + Eterm sysname = dep->sysname; + enum dist_entry_state state = dep->state; + + if (dep->state != ERTS_DE_STATE_PENDING) + ERTS_INTERNAL_ERROR("Garbage collecting connected distribution entry"); + erts_abort_connection_rwunlock(dep); + report_gc_active_dist_entry(sysname, state); + } + else + erts_de_rwunlock(dep); + erts_rwmtx_rwlock(&erts_dist_table_rwmtx); /* * Another thread might have looked up this dist entry after @@ -515,6 +494,34 @@ static void try_delete_dist_entry(DistEntry* dep) } } +static void report_gc_active_dist_entry(Eterm sysname, + enum dist_entry_state state) +{ + char *state_str; + erts_dsprintf_buf_t *dsbuf = erts_create_logger_dsbuf(); + switch (state) { + case ERTS_DE_STATE_CONNECTED: + state_str = "connected"; + break; + case ERTS_DE_STATE_PENDING: + state_str = "pending connect"; + break; + case ERTS_DE_STATE_EXITING: + state_str = "exiting"; + break; + case ERTS_DE_STATE_IDLE: + state_str = "idle"; + break; + default: + state_str = "unknown"; + break; + } + erts_dsprintf(dsbuf, "Garbage collecting distribution " + "entry for node %T in state: %s", + sysname, state_str); + erts_send_error_to_logger_nogl(dsbuf); +} + int erts_dist_entry_destructor(Binary *bin) { DistEntry *dep = ErtsBin2DistEntry(bin); -- cgit v1.2.3