From cb2a5bd9e86ba49d9bbc83b3d8383fbe0cc90715 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Tue, 14 Feb 2017 19:25:16 +0100 Subject: erts: Avoid revival of dying resource by dec_term --- erts/emulator/beam/erl_binary.h | 8 ++- erts/emulator/beam/erl_nif.c | 88 ++++++++++++++++----------------- erts/emulator/beam/global.h | 12 ++--- erts/emulator/sys/common/erl_check_io.c | 2 +- 4 files changed, 54 insertions(+), 56 deletions(-) (limited to 'erts/emulator') diff --git a/erts/emulator/beam/erl_binary.h b/erts/emulator/beam/erl_binary.h index db259be2a7..946d3cfe03 100644 --- a/erts/emulator/beam/erl_binary.h +++ b/erts/emulator/beam/erl_binary.h @@ -79,11 +79,9 @@ struct magic_binary { } u; }; -#ifdef ARCH_32 -#define ERTS_MAGIC_BIN_BYTES_TO_ALIGN 4 -#else -#define ERTS_MAGIC_BIN_BYTES_TO_ALIGN 0 -#endif +#define ERTS_MAGIC_BIN_BYTES_TO_ALIGN \ + (offsetof(ErtsMagicBinary,u.aligned.data) - \ + offsetof(ErtsMagicBinary,u.unaligned.data)) typedef union { Binary binary; diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c index 7e7b78ec14..daec4ac9e9 100644 --- a/erts/emulator/beam/erl_nif.c +++ b/erts/emulator/beam/erl_nif.c @@ -2141,7 +2141,7 @@ static void rollback_opened_resource_types(void) struct destroy_monitor_ctx { - Binary* resource_bin; + ErtsResource* resource; int exiting_procs; int scheduler; }; @@ -2188,30 +2188,26 @@ static void destroy_one_monitor(ErtsMonitor* mon, void* context) #endif } if (is_exiting) { - ctx->exiting_procs++; - /* +1 for exiting process */ - erts_refc_inc(&ctx->resource_bin->refc, ctx->exiting_procs); + ctx->resource->monitors->pending_failed_fire++; } /* ToDo: Delay destruction after monitor_locks */ if (rmon) { ASSERT(rmon->type == MON_NIF_TARGET); - ASSERT(&ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(rmon->u.pid)->binary == ctx->resource_bin); + ASSERT(rmon->u.resource == ctx->resource); erts_destroy_monitor(rmon); } erts_destroy_monitor(mon); } -static int destroy_all_monitors(ErtsMonitor* monitors, Binary* bin) +static void destroy_all_monitors(ErtsMonitor* monitors, ErtsResource* resource) { struct destroy_monitor_ctx ctx; execution_state(NULL, NULL, &ctx.scheduler); - ctx.resource_bin = bin; - ctx.exiting_procs = 0; + ctx.resource = resource; erts_sweep_monitors(monitors, &destroy_one_monitor, &ctx); - return ctx.exiting_procs == 0; } @@ -2252,31 +2248,32 @@ static int nif_resource_dtor(Binary* bin) if (resource->monitors) { ErtsResourceMonitors* rm = resource->monitors; - ErtsMonitor* root; ASSERT(type->down); erts_smp_mtx_lock(&rm->lock); ASSERT(erts_refc_read(&bin->refc, 0) == 0); - root = rm->root; - if (root) { + if (rm->root) { + ASSERT(!rm->is_dying); + destroy_all_monitors(rm->root, resource); rm->root = NULL; - if (!destroy_all_monitors(root, bin)) { - /* - * Resource death struggle prolonged to serve exiting process(es). - * Destructor will be called again when last exiting process - * tries to fire its MON_NIF_TARGET monitor (and fails). - * - * This resource is doomed. It has no "real" references and - * should get not get called upon to do anything except the - * final destructor call. - */ - ASSERT(erts_refc_read(&bin->refc, 1)); -#ifdef DEBUG - resource->dbg_is_dying = 1; -#endif - erts_smp_mtx_unlock(&rm->lock); - return 0; - } + } + if (rm->pending_failed_fire) { + /* + * Resource death struggle prolonged to serve exiting process(es). + * Destructor will be called again when last exiting process + * tries to fire its MON_NIF_TARGET monitor (and fails). + * + * This resource is doomed. It has no "real" references and + * should get not get called upon to do anything except the + * final destructor call. + * + * We keep refc at 0 and use a separate counter for exiting + * processes to avoid resource getting revived by "dec_term". + */ + ASSERT(!rm->is_dying); + rm->is_dying = 1; + erts_smp_mtx_unlock(&rm->lock); + return 0; } erts_smp_mtx_unlock(&rm->lock); erts_smp_mtx_destroy(&rm->lock); @@ -2326,24 +2323,24 @@ void erts_fire_nif_monitor(ErtsResource* resource, Eterm pid, Eterm ref) erts_smp_mtx_lock(&rmp->lock); rmon = erts_remove_monitor(&rmp->root, ref); if (!rmon) { + int free_me = (--rmp->pending_failed_fire == 0) && rmp->is_dying; + ASSERT(rmp->pending_failed_fire >= 0); erts_smp_mtx_unlock(&rmp->lock); - /* -1 for exiting process */ - if (erts_refc_dectest(&bin->binary.refc, 0) == 0) { + + if (free_me) { + ASSERT(erts_refc_read(&bin->binary.refc, 0) == 0); erts_bin_free(&bin->binary); } return; } - ASSERT(!resource->dbg_is_dying); - if (erts_refc_inctest(&bin->binary.refc, 1) < 2) { + ASSERT(!rmp->is_dying); + if (erts_refc_inc_unless(&bin->binary.refc, 0, 0) == 0) { /* * Racing resource destruction. * To avoid a more complex refc-dance with destructing thread * we avoid calling 'down' and just silently remove the monitor. - * There are no real references left to this resource and we have - * monitors_lock, so it's safe to reset refc back to zero. * This can happen even for non smp as destructor calls may be scheduled. */ - erts_refc_init(&bin->binary.refc, 0); erts_smp_mtx_unlock(&rmp->lock); } else { @@ -2390,13 +2387,14 @@ void* enif_alloc_resource(ErlNifResourceType* type, size_t data_sz) erts_refc_inc(&bin->refc, 1); #ifdef DEBUG erts_refc_init(&resource->nif_refc, 1); - resource->dbg_is_dying = 0; #endif erts_refc_inc(&resource->type->refc, 2); if (type->down) { resource->monitors = (ErtsResourceMonitors*) (resource->data + monitors_offs); erts_smp_mtx_init(&resource->monitors->lock, "resource_monitors"); resource->monitors->root = NULL; + resource->monitors->pending_failed_fire = 0; + resource->monitors->is_dying = 0; resource->monitors->user_data_sz = data_sz; } else { @@ -2411,7 +2409,7 @@ void enif_release_resource(void* obj) ErtsBinary* bin = ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(resource); ASSERT(ERTS_MAGIC_BIN_DESTRUCTOR(bin) == NIF_RESOURCE_DTOR); - ASSERT(!resource->dbg_is_dying); + ASSERT(!(resource->monitors && resource->monitors->is_dying)); #ifdef DEBUG erts_refc_dec(&resource->nif_refc, 0); #endif @@ -2426,7 +2424,7 @@ void enif_keep_resource(void* obj) ErtsBinary* bin = ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(resource); ASSERT(ERTS_MAGIC_BIN_DESTRUCTOR(bin) == NIF_RESOURCE_DTOR); - ASSERT(!resource->dbg_is_dying); + ASSERT(!(resource->monitors && resource->monitors->is_dying)); #ifdef DEBUG erts_refc_inc(&resource->nif_refc, 1); #endif @@ -2436,7 +2434,7 @@ void enif_keep_resource(void* obj) Eterm erts_bld_resource_ref(Eterm** hpp, ErlOffHeap* oh, ErtsResource* resource) { ErtsBinary* bin = ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(resource); - ASSERT(!resource->dbg_is_dying); + ASSERT(!(resource->monitors && resource->monitors->is_dying)); return erts_mk_magic_ref(hpp, oh, &bin->binary); } @@ -2445,6 +2443,7 @@ ERL_NIF_TERM enif_make_resource(ErlNifEnv* env, void* obj) ErtsResource* resource = DATA_TO_RESOURCE(obj); ErtsBinary* bin = ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(resource); Eterm* hp = alloc_heap(env, ERTS_MAGIC_REF_THING_SIZE); + ASSERT(!(resource->monitors && resource->monitors->is_dying)); return erts_mk_magic_ref(&hp, &MSO(env->proc), &bin->binary); } @@ -3136,7 +3135,7 @@ int enif_monitor_process(ErlNifEnv* env, void* obj, const ErlNifPid* target_pid, ASSERT(ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(rsrc)->magic_binary.destructor == NIF_RESOURCE_DTOR); - ASSERT(!rsrc->dbg_is_dying); + ASSERT(!(rsrc->monitors && rsrc->monitors->is_dying)); ASSERT(!rsrc->monitors == !rsrc->type->down); @@ -3193,7 +3192,9 @@ int enif_demonitor_process(ErlNifEnv* env, void* obj, const ErlNifMonitor* monit { int scheduler; ErtsResource* rsrc = DATA_TO_RESOURCE(obj); +#ifdef DEBUG ErtsBinary* bin = ERTS_MAGIC_BIN_FROM_UNALIGNED_DATA(rsrc); +#endif Process *rp; ErtsMonitor *mon; ErtsMonitor *rmon = NULL; @@ -3202,7 +3203,7 @@ int enif_demonitor_process(ErlNifEnv* env, void* obj, const ErlNifMonitor* monit int is_exiting; ASSERT(bin->magic_binary.destructor == NIF_RESOURCE_DTOR); - ASSERT(!rsrc->dbg_is_dying); + ASSERT(!(rsrc->monitors && rsrc->monitors->is_dying)); execution_state(env, NULL, &scheduler); @@ -3252,8 +3253,7 @@ int enif_demonitor_process(ErlNifEnv* env, void* obj, const ErlNifMonitor* monit #endif } if (is_exiting) { - /* +1 for exiting process */ - erts_refc_inc(&bin->binary.refc, 2); + rsrc->monitors->pending_failed_fire++; } erts_smp_mtx_unlock(&rsrc->monitors->lock); diff --git a/erts/emulator/beam/global.h b/erts/emulator/beam/global.h index d4a1225bdd..776f2c599b 100644 --- a/erts/emulator/beam/global.h +++ b/erts/emulator/beam/global.h @@ -87,6 +87,9 @@ typedef struct { erts_smp_mtx_t lock; ErtsMonitor* root; + int pending_failed_fire; + int is_dying; + size_t user_data_sz; } ErtsResourceMonitors; @@ -94,14 +97,11 @@ typedef struct ErtsResource_ { struct enif_resource_type_t* type; ErtsResourceMonitors* monitors; -#ifdef ARCH_32 - byte align__[4]; -#endif #ifdef DEBUG erts_refc_t nif_refc; - int dbg_is_dying; -# ifdef ARCH_64 - byte dbg_align__[4]; +#else +# ifdef ARCH_32 + byte align__[4]; # endif #endif char data[1]; diff --git a/erts/emulator/sys/common/erl_check_io.c b/erts/emulator/sys/common/erl_check_io.c index 2214a1937a..1c97df4201 100644 --- a/erts/emulator/sys/common/erl_check_io.c +++ b/erts/emulator/sys/common/erl_check_io.c @@ -1223,7 +1223,7 @@ ERTS_CIO_EXPORT(enif_select)(ErlNifEnv* env, DTRACE_CHARBUF(name, 64); #endif - ASSERT(!resource->dbg_is_dying); + ASSERT(!(resource->monitors && resource->monitors->is_dying)); #ifdef ERTS_SYS_CONTINOUS_FD_NUMBERS if ((unsigned)fd >= (unsigned)erts_smp_atomic_read_nob(&drv_ev_state_len)) { -- cgit v1.2.3