aboutsummaryrefslogtreecommitdiffstats
path: root/erts/emulator
diff options
context:
space:
mode:
authorSverker Eriksson <[email protected]>2014-02-05 19:41:28 +0100
committerSverker Eriksson <[email protected]>2014-02-05 19:41:28 +0100
commit4082c3d1ad3d0e8bb85706c5f3b2bde97bb43fcb (patch)
tree4f70a7a70457aacaf96b7e1a7399945a67d27ba5 /erts/emulator
parentab20369b433a8e6440e5e71a1c92bf7440916192 (diff)
downloadotp-4082c3d1ad3d0e8bb85706c5f3b2bde97bb43fcb.tar.gz
otp-4082c3d1ad3d0e8bb85706c5f3b2bde97bb43fcb.tar.bz2
otp-4082c3d1ad3d0e8bb85706c5f3b2bde97bb43fcb.zip
erts: Fix NIF bug when load/upgrade fails after enif_open_resource_type
..has been successfully called. Opened resource types (created or taken-over) were left "hanging" leading both to memory leakage and other more strange and serious behavior. Now a proper rollback is done.
Diffstat (limited to 'erts/emulator')
-rw-r--r--erts/emulator/beam/erl_nif.c113
-rw-r--r--erts/emulator/test/nif_SUITE.erl136
-rw-r--r--erts/emulator/test/nif_SUITE_data/nif_SUITE.c3
-rw-r--r--erts/emulator/test/nif_SUITE_data/nif_mod.c47
4 files changed, 257 insertions, 42 deletions
diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index dc285b3cf7..b2fd3dcd24 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -1,7 +1,7 @@
/*
* %CopyrightBegin%
*
- * Copyright Ericsson AB 2009-2013. All Rights Reserved.
+ * Copyright Ericsson AB 2009-2014. All Rights Reserved.
*
* The contents of this file are subject to the Erlang Public License,
* Version 1.1, (the "License"); you may not use this file except in
@@ -1235,6 +1235,19 @@ static void steal_resource_type(ErlNifResourceType* type)
}
}
+/* The opened_rt_list is used by enif_open_resource_type()
+ * in order to rollback "creates" and "take-overs" in case the load fails.
+ */
+struct opened_resource_type
+{
+ struct opened_resource_type* next;
+
+ ErlNifResourceFlags op;
+ ErlNifResourceType* type;
+ ErlNifResourceDtor* new_dtor;
+};
+static struct opened_resource_type* opened_rt_list = NULL;
+
ErlNifResourceType*
enif_open_resource_type(ErlNifEnv* env,
const char* module_str,
@@ -1256,22 +1269,21 @@ enif_open_resource_type(ErlNifEnv* env,
if (type == NULL) {
if (flags & ERL_NIF_RT_CREATE) {
type = erts_alloc(ERTS_ALC_T_NIF,
- sizeof(struct enif_resource_type_t));
- type->dtor = dtor;
+ sizeof(struct enif_resource_type_t));
type->module = module_am;
type->name = name_am;
erts_refc_init(&type->refc, 1);
- type->owner = env->mod_nif;
- type->prev = &resource_type_list;
- type->next = resource_type_list.next;
- type->next->prev = type;
- type->prev->next = type;
op = ERL_NIF_RT_CREATE;
+ #ifdef DEBUG
+ type->dtor = (void*)1;
+ type->owner = (void*)2;
+ type->prev = (void*)3;
+ type->next = (void*)4;
+ #endif
}
}
else {
- if (flags & ERL_NIF_RT_TAKEOVER) {
- steal_resource_type(type);
+ if (flags & ERL_NIF_RT_TAKEOVER) {
op = ERL_NIF_RT_TAKEOVER;
}
else {
@@ -1279,12 +1291,13 @@ enif_open_resource_type(ErlNifEnv* env,
}
}
if (type != NULL) {
- type->owner = env->mod_nif;
- type->dtor = dtor;
- if (type->dtor != NULL) {
- erts_refc_inc(&type->owner->rt_dtor_cnt, 1);
- }
- erts_refc_inc(&type->owner->rt_cnt, 1);
+ struct opened_resource_type* ort = erts_alloc(ERTS_ALC_T_TMP,
+ sizeof(struct opened_resource_type));
+ ort->op = op;
+ ort->type = type;
+ ort->new_dtor = dtor;
+ ort->next = opened_rt_list;
+ opened_rt_list = ort;
}
if (tried != NULL) {
*tried = op;
@@ -1292,6 +1305,51 @@ enif_open_resource_type(ErlNifEnv* env,
return type;
}
+static void commit_opened_resource_types(struct erl_module_nif* lib)
+{
+ while (opened_rt_list) {
+ struct opened_resource_type* ort = opened_rt_list;
+
+ ErlNifResourceType* type = ort->type;
+
+ if (ort->op == ERL_NIF_RT_CREATE) {
+ type->prev = &resource_type_list;
+ type->next = resource_type_list.next;
+ type->next->prev = type;
+ type->prev->next = type;
+ }
+ else { /* ERL_NIF_RT_TAKEOVER */
+ steal_resource_type(type);
+ }
+
+ type->owner = lib;
+ type->dtor = ort->new_dtor;
+
+ if (type->dtor != NULL) {
+ erts_refc_inc(&lib->rt_dtor_cnt, 1);
+ }
+ erts_refc_inc(&lib->rt_cnt, 1);
+
+ opened_rt_list = ort->next;
+ erts_free(ERTS_ALC_T_TMP, ort);
+ }
+}
+
+static void rollback_opened_resource_types(void)
+{
+ while (opened_rt_list) {
+ struct opened_resource_type* ort = opened_rt_list;
+
+ if (ort->op == ERL_NIF_RT_CREATE) {
+ erts_free(ERTS_ALC_T_NIF, ort->type);
+ }
+
+ opened_rt_list = ort->next;
+ erts_free(ERTS_ALC_T_TMP, ort);
+ }
+}
+
+
static void nif_resource_dtor(Binary* bin)
{
ErlNifResource* resource = (ErlNifResource*) ERTS_MAGIC_BIN_DATA(bin);
@@ -1317,6 +1375,8 @@ void* enif_alloc_resource(ErlNifResourceType* type, size_t size)
{
Binary* bin = erts_create_magic_binary(SIZEOF_ErlNifResource(size), &nif_resource_dtor);
ErlNifResource* resource = ERTS_MAGIC_BIN_DATA(bin);
+
+ ASSERT(type->owner && type->next && type->prev); /* not allowed in load/upgrade */
resource->type = type;
erts_refc_inc(&bin->refc, 1);
#ifdef DEBUG
@@ -1696,9 +1756,15 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
lib->entry = entry;
erts_refc_init(&lib->rt_cnt, 0);
erts_refc_init(&lib->rt_dtor_cnt, 0);
+ ASSERT(opened_rt_list == NULL);
lib->mod = mod;
env.mod_nif = lib;
- if (mod->curr.nif != NULL) { /* Reload */
+ if (mod->curr.nif != NULL) { /*************** Reload ******************/
+ /*
+ * Repeated load_nif calls from same Erlang module instance ("reload")
+ * is deprecated and was only ment as a development feature not to
+ * be used in production systems. (See warning below)
+ */
int k;
lib->priv_data = mod->curr.nif->priv_data;
@@ -1730,6 +1796,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
ret = load_nif_error(BIF_P, reload, "Library reload-call unsuccessful.");
}
else {
+ commit_opened_resource_types(lib);
mod->curr.nif->entry = NULL; /* to prevent 'unload' callback */
erts_unload_nif(mod->curr.nif);
reload_warning = 1;
@@ -1737,7 +1804,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
}
else {
lib->priv_data = NULL;
- if (mod->old.nif != NULL) { /* Upgrade */
+ if (mod->old.nif != NULL) { /**************** Upgrade ***************/
void* prev_old_data = mod->old.nif->priv_data;
if (entry->upgrade == NULL) {
ret = load_nif_error(BIF_P, upgrade, "Upgrade not supported by this NIF library.");
@@ -1750,17 +1817,18 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
mod->old.nif->priv_data = prev_old_data;
ret = load_nif_error(BIF_P, upgrade, "Library upgrade-call unsuccessful.");
}
- /*else if (mod->old_nif->priv_data != prev_old_data) {
- refresh_cached_nif_data(mod->old_code, mod->old_nif);
- }*/
+ else
+ commit_opened_resource_types(lib);
}
- else if (entry->load != NULL) { /* Initial load */
+ else if (entry->load != NULL) { /********* Initial load ***********/
erts_pre_nif(&env, BIF_P, lib);
veto = entry->load(&env, &lib->priv_data, BIF_ARG_2);
erts_post_nif(&env);
if (veto) {
ret = load_nif_error(BIF_P, "load", "Library load-call unsuccessful.");
}
+ else
+ commit_opened_resource_types(lib);
}
}
if (ret == am_ok) {
@@ -1789,6 +1857,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2)
}
else {
error:
+ rollback_opened_resource_types();
ASSERT(ret != am_ok);
if (lib != NULL) {
erts_free(ERTS_ALC_T_NIF, lib);
diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl
index 9a70e8646a..c88fa4b228 100644
--- a/erts/emulator/test/nif_SUITE.erl
+++ b/erts/emulator/test/nif_SUITE.erl
@@ -1,7 +1,7 @@
%%
%% %CopyrightBegin%
%%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2014. All Rights Reserved.
%%
%% The contents of this file are subject to the Erlang Public License,
%% Version 1.1, (the "License"); you may not use this file except in
@@ -801,6 +801,138 @@ resource_takeover(Config) when is_list(Config) ->
?line ok = forget_resource(AN4),
?line [] = nif_mod_call_history(),
+
+ %%
+ %% Test rollback after failed upgrade of same lib-version
+ %%
+
+ {A5,BinA5} = make_resource(2, Holder, "A5"),
+ {NA5,BinNA5} = make_resource(0, Holder, "NA5"),
+ {AN5,_BinAN5} = make_resource(1, Holder, "AN5"),
+
+ {A6,BinA6} = make_resource(2, Holder, "A6"),
+ {NA6,BinNA6} = make_resource(0, Holder, "NA6"),
+ {AN6,_BinAN6} = make_resource(1, Holder, "AN6"),
+
+ {module,nif_mod} = erlang:load_module(nif_mod,ModBin),
+ undefined = nif_mod:lib_version(),
+ {error,{upgrade,_}} =
+ nif_mod:load_nif_lib(Config, 1,
+ [{resource_type, 4, ?RT_TAKEOVER, "resource_type_A",resource_dtor_B,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",null,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_TAKEOVER, "resource_type_A_null",resource_dtor_A,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_CREATE, "Mr Pink", resource_dtor_A,
+ ?RT_CREATE},
+
+ {return, 1} % FAIL
+ ]),
+
+ undefined = nif_mod:lib_version(),
+ [{upgrade,1,5,105}] = nif_mod_call_history(),
+
+ %% Make sure dtor was not changed (from A to B)
+ ok = forget_resource(A5),
+ [{{resource_dtor_A_v1,BinA5},1,6,106}] = nif_mod_call_history(),
+
+ %% Make sure dtor was not nullified (from A to null)
+ ok = forget_resource(NA5),
+ [{{resource_dtor_A_v1,BinNA5},1,7,107}] = nif_mod_call_history(),
+
+ %% Make sure dtor was not added (from null to A)
+ ok = forget_resource(AN5),
+ [] = nif_mod_call_history(),
+
+ %%
+ %% Test rollback after failed upgrade of other lib-version
+ %%
+
+ {error,{upgrade,_}} =
+ nif_mod:load_nif_lib(Config, 2,
+ [{resource_type, 4, ?RT_TAKEOVER, "resource_type_A",resource_dtor_B,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",null,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_TAKEOVER, "resource_type_A_null",resource_dtor_A,
+ ?RT_TAKEOVER},
+ {resource_type, null, ?RT_TAKEOVER, "Mr Pink", resource_dtor_A,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_CREATE, "Mr Pink", resource_dtor_A,
+ ?RT_CREATE},
+
+ {return, 1} % FAIL
+ ]),
+
+ undefined = nif_mod:lib_version(),
+ [{upgrade,2,_,_}] = nif_mod_call_history(),
+
+ %% Make sure dtor was not changed (from A to B)
+ ok = forget_resource(A6),
+ [{{resource_dtor_A_v1,BinA6},1,_,_}] = nif_mod_call_history(),
+
+ %% Make sure dtor was not nullified (from A to null)
+ ok = forget_resource(NA6),
+ [{{resource_dtor_A_v1,BinNA6},1,_,_}] = nif_mod_call_history(),
+
+ %% Make sure dtor was not added (from null to A)
+ ok = forget_resource(AN6),
+ [] = nif_mod_call_history(),
+
+ %%
+ %% Test rolback after failed initial load
+ %%
+ false = code:purge(nif_mod),
+ [{unload,1,_,_}] = nif_mod_call_history(),
+ true = code:delete(nif_mod),
+ false = code:purge(nif_mod),
+ [] = nif_mod_call_history(),
+
+
+ {module,nif_mod} = erlang:load_module(nif_mod,ModBin),
+ undefined = nif_mod:lib_version(),
+ {error,{load,_}} =
+ nif_mod:load_nif_lib(Config, 1,
+ [{resource_type, null, ?RT_TAKEOVER, "resource_type_A",resource_dtor_A,
+ ?RT_TAKEOVER},
+ {resource_type, 4, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",null,
+ ?RT_CREATE},
+ {resource_type, 4, ?RT_CREATE, "resource_type_A_null",resource_dtor_A,
+ ?RT_CREATE},
+ {resource_type, 4, ?RT_CREATE, "Mr Pink", resource_dtor_A,
+ ?RT_CREATE},
+
+ {return, 1} % FAIL
+ ]),
+
+ undefined = nif_mod:lib_version(),
+ ok = nif_mod:load_nif_lib(Config, 1,
+ [{resource_type, null, ?RT_TAKEOVER, "resource_type_A",resource_dtor_A,
+ ?RT_TAKEOVER},
+ {resource_type, 0, ?RT_TAKEOVER bor ?RT_CREATE, "resource_type_null_A",
+ resource_dtor_A, ?RT_CREATE},
+
+ {resource_type, 1, ?RT_CREATE, "resource_type_A_null", null,
+ ?RT_CREATE},
+ {resource_type, null, ?RT_TAKEOVER, "Mr Pink", resource_dtor_A,
+ ?RT_TAKEOVER},
+
+ {return, 0} % SUCCESS
+ ]),
+
+ ?line hold_nif_mod_priv_data(nif_mod:get_priv_data_ptr()),
+ ?line [{load,1,1,101},{get_priv_data_ptr,1,2,102}] = nif_mod_call_history(),
+
+ {NA7,BinNA7} = make_resource(0, Holder, "NA7"),
+ {AN7,BinAN7} = make_resource(1, Holder, "AN7"),
+
+ ok = forget_resource(NA7),
+ [{{resource_dtor_A_v1,BinNA7},1,_,_}] = nif_mod_call_history(),
+
+ ok = forget_resource(AN7),
+ [] = nif_mod_call_history(),
+
?line true = lists:member(?MODULE, erlang:system_info(taints)),
?line true = lists:member(nif_mod, erlang:system_info(taints)),
?line verify_tmpmem(TmpMem),
@@ -1343,7 +1475,7 @@ consume_timeslice(Config) when is_list(Config) ->
ok.
-next_msg(Pid) ->
+next_msg(_Pid) ->
receive
M -> M
after 100 ->
diff --git a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
index 0c4a9f7e5c..cdc64b89b2 100644
--- a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
+++ b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
@@ -1,7 +1,7 @@
/*
* %CopyrightBegin%
*
- * Copyright Ericsson AB 2009-2013. All Rights Reserved.
+ * Copyright Ericsson AB 2009-2014. All Rights Reserved.
*
* The contents of this file are subject to the Erlang Public License,
* Version 1.1, (the "License"); you may not use this file except in
@@ -1477,7 +1477,6 @@ static ERL_NIF_TERM consume_timeslice_nif(ErlNifEnv* env, int argc, const ERL_NI
{
int percent;
char atom[10];
- int do_repeat;
if (!enif_get_int(env, argv[0], &percent) ||
!enif_get_atom(env, argv[1], atom, sizeof(atom), ERL_NIF_LATIN1)) {
diff --git a/erts/emulator/test/nif_SUITE_data/nif_mod.c b/erts/emulator/test/nif_SUITE_data/nif_mod.c
index e32d10057c..aed8524635 100644
--- a/erts/emulator/test/nif_SUITE_data/nif_mod.c
+++ b/erts/emulator/test/nif_SUITE_data/nif_mod.c
@@ -1,7 +1,7 @@
/*
* %CopyrightBegin%
*
- * Copyright Ericsson AB 2009-2010. All Rights Reserved.
+ * Copyright Ericsson AB 2009-2014. All Rights Reserved.
*
* The contents of this file are subject to the Erlang Public License,
* Version 1.1, (the "License"); you may not use this file except in
@@ -41,6 +41,7 @@ static ERL_NIF_TERM am_null;
static ERL_NIF_TERM am_resource_type;
static ERL_NIF_TERM am_resource_dtor_A;
static ERL_NIF_TERM am_resource_dtor_B;
+static ERL_NIF_TERM am_return;
static NifModPrivData* priv_data(ErlNifEnv* env)
{
@@ -54,6 +55,7 @@ static void init(ErlNifEnv* env)
am_resource_type = enif_make_atom(env, "resource_type");
am_resource_dtor_A = enif_make_atom(env, "resource_dtor_A");
am_resource_dtor_B = enif_make_atom(env, "resource_dtor_B");
+ am_return = enif_make_atom(env, "return");
}
static void add_call_with_arg(ErlNifEnv* env, NifModPrivData* data, const char* func_name,
@@ -105,19 +107,15 @@ static void resource_dtor_B(ErlNifEnv* env, void* a)
}
/* {resource_type, Ix|null, ErlNifResourceFlags in, "TypeName", dtor(A|B|null), ErlNifResourceFlags out}*/
-static void open_resource_type(ErlNifEnv* env, ERL_NIF_TERM op_tpl)
+static void open_resource_type(ErlNifEnv* env, const ERL_NIF_TERM* arr)
{
NifModPrivData* data = priv_data(env);
- const ERL_NIF_TERM* arr;
- int arity;
char rt_name[30];
union { ErlNifResourceFlags e; int i; } flags, exp_res, got_res;
unsigned ix;
ErlNifResourceDtor* dtor;
ErlNifResourceType* got_ptr;
- CHECK(enif_get_tuple(env, op_tpl, &arity, &arr));
- CHECK(arity == 6);
CHECK(enif_is_identical(arr[0], am_resource_type));
CHECK(enif_get_int(env, arr[2], &flags.i));
CHECK(enif_get_string(env, arr[3], rt_name, sizeof(rt_name), ERL_NIF_LATIN1) > 0);
@@ -147,18 +145,32 @@ static void open_resource_type(ErlNifEnv* env, ERL_NIF_TERM op_tpl)
CHECK(got_res.e == exp_res.e);
}
-static void do_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info)
+static void do_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info, int* retvalp)
{
NifModPrivData* data = priv_data(env);
ERL_NIF_TERM head, tail;
unsigned ix;
+
for (ix=0; ix<RT_MAX; ix++) {
data->rt_arr[ix] = NULL;
}
for (head = load_info; enif_get_list_cell(env, head, &head, &tail);
head = tail) {
-
- open_resource_type(env, head);
+ const ERL_NIF_TERM* arr;
+ int arity;
+
+ CHECK(enif_get_tuple(env, head, &arity, &arr));
+ switch (arity) {
+ case 6:
+ open_resource_type(env, arr);
+ break;
+ case 2:
+ CHECK(arr[0] == am_return);
+ CHECK(enif_get_int(env, arr[1], retvalp));
+ break;
+ default:
+ CHECK(0);
+ }
}
CHECK(enif_is_empty_list(env, head));
}
@@ -166,6 +178,7 @@ static void do_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info)
static int load(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info)
{
NifModPrivData* data;
+ int retval = 0;
init(env);
data = (NifModPrivData*) enif_alloc(sizeof(NifModPrivData));
@@ -177,38 +190,40 @@ static int load(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info)
add_call(env, data, "load");
- do_load_info(env, load_info);
+ do_load_info(env, load_info, &retval);
data->calls = 0;
- return 0;
+ return retval;
}
static int reload(ErlNifEnv* env, void** priv, ERL_NIF_TERM load_info)
{
NifModPrivData* data = (NifModPrivData*) *priv;
+ int retval = 0;
init(env);
add_call(env, data, "reload");
- do_load_info(env, load_info);
- return 0;
+ do_load_info(env, load_info, &retval);
+ return retval;
}
static int upgrade(ErlNifEnv* env, void** priv, void** old_priv_data, ERL_NIF_TERM load_info)
{
NifModPrivData* data = (NifModPrivData*) *old_priv_data;
+ int retval = 0;
init(env);
add_call(env, data, "upgrade");
data->ref_cnt++;
*priv = *old_priv_data;
- do_load_info(env, load_info);
+ do_load_info(env, load_info, &retval);
- return 0;
+ return retval;
}
static void unload(ErlNifEnv* env, void* priv)
{
NifModPrivData* data = (NifModPrivData*) priv;
- int is_last;
+
add_call(env, data, "unload");
NifModPrivData_release(data);
}