From 3b513ac820ab529a565d8caf3972982f204e8b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Mon, 2 May 2016 14:53:33 +0200 Subject: code_SUITE: Make on_load_binary/1 clearer by using merl --- lib/kernel/test/code_SUITE.erl | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/lib/kernel/test/code_SUITE.erl b/lib/kernel/test/code_SUITE.erl index 383eab94fe..a0b72925f7 100644 --- a/lib/kernel/test/code_SUITE.erl +++ b/lib/kernel/test/code_SUITE.erl @@ -1184,22 +1184,17 @@ on_load_binary(_) -> register(Master, self()), %% Construct, compile and pretty-print. - Mod = on_load_binary, + Mod = ?FUNCTION_NAME, File = atom_to_list(Mod) ++ ".erl", - Forms = [{attribute,1,file,{File,1}}, - {attribute,1,module,Mod}, - {attribute,2,export,[{ok,0}]}, - {attribute,3,on_load,{init,0}}, - {function,5,init,0, - [{clause,5,[],[], - [{op,6,'!', - {atom,6,Master}, - {tuple,6,[{atom,6,Mod},{call,6,{atom,6,self},[]}]}}, - {'receive',7,[{clause,8,[{atom,8,go}],[],[{atom,8,ok}]}]}]}]}, - {function,11,ok,0,[{clause,11,[],[],[{atom,11,true}]}]}], - Forms1 = erl_parse:new_anno(Forms), - {ok,Mod,Bin} = compile:forms(Forms1, [report]), - [io:put_chars(erl_pp:form(F)) || F <- Forms1], + Tree = ?Q(["-module('@Mod@').\n", + "-export([ok/0]).\n", + "-on_load({init,0}).\n", + "init() ->\n", + " '@Master@' ! {on_load_binary,self()},\n", + " receive go -> ok end.\n", + "ok() -> true.\n"]), + {ok,Mod,Bin} = merl:compile(Tree), + merl:print(Tree), {Pid1,Ref1} = spawn_monitor(fun() -> code:load_binary(Mod, File, Bin), -- cgit v1.2.3 From 4050fc5665a4819b15f204543c28c59726b4aac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 12 Apr 2016 07:07:17 +0200 Subject: Refactor erts_finish_loading() and insert_new_code() As a preparation for fixing some issues with -on_load(), we will integrate insert_new_code() into erts_finish_loading. Also rename insert_new_code() to stub_insert_new_code() to make it clear that it will only be used when making a stub module --- erts/emulator/beam/beam_load.c | 53 +++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index 2e21a553ed..9bd9b4bfc0 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -479,9 +479,9 @@ static void free_loader_state(Binary* magic); static ErlHeapFragment* new_literal_fragment(Uint size); static void free_literal_fragment(ErlHeapFragment*); static void loader_state_dtor(Binary* magic); -static Eterm insert_new_code(Process *c_p, ErtsProcLocks c_p_locks, - Eterm group_leader, Eterm module, - BeamCodeHeader* code, Uint size); +static Eterm stub_insert_new_code(Process *c_p, ErtsProcLocks c_p_locks, + Eterm group_leader, Eterm module, + BeamCodeHeader* code, Uint size); static int init_iff_file(LoaderState* stp, byte* code, Uint size); static int scan_iff_file(LoaderState* stp, Uint* chunk_types, Uint num_types, Uint num_mandatory); @@ -513,7 +513,7 @@ static GenOp* gen_get_map_element(LoaderState* stp, GenOpArg Fail, GenOpArg Src, static int freeze_code(LoaderState* stp); -static void final_touch(LoaderState* stp); +static void final_touch(LoaderState* stp, struct erl_module_instance* inst_p); static void short_file(int line, LoaderState* stp, unsigned needed); static void load_printf(int line, LoaderState* context, char *fmt, ...); static int transform_engine(LoaderState* st); @@ -768,6 +768,9 @@ erts_finish_loading(Binary* magic, Process* c_p, { Eterm retval; LoaderState* stp = ERTS_MAGIC_BIN_DATA(magic); + Module* mod_tab_p; + struct erl_module_instance* inst_p; + Uint size; /* * No other process may run since we will update the export @@ -784,11 +787,25 @@ erts_finish_loading(Binary* magic, Process* c_p, */ CHKBLK(ERTS_ALC_T_CODE,stp->code); - retval = insert_new_code(c_p, c_p_locks, stp->group_leader, stp->module, - stp->hdr, stp->loaded_size); - if (retval != NIL) { - goto load_error; - } + retval = beam_make_current_old(c_p, c_p_locks, stp->module); + ASSERT(retval == NIL); + + /* + * Update module table. + */ + + size = stp->loaded_size; + erts_total_code_size += size; + mod_tab_p = erts_put_module(stp->module); + inst_p = &mod_tab_p->curr; + inst_p->code_hdr = stp->hdr; + inst_p->code_length = size; + + /* + * Update ranges (used for finding a function from a PC value). + */ + + erts_update_ranges((BeamInstr*)inst_p->code_hdr, size); /* * Ready for the final touch: fixing the export table entries for @@ -796,7 +813,7 @@ erts_finish_loading(Binary* magic, Process* c_p, */ CHKBLK(ERTS_ALC_T_CODE,stp->code); - final_touch(stp); + final_touch(stp, inst_p); /* * Loading succeded. @@ -820,7 +837,6 @@ erts_finish_loading(Binary* magic, Process* c_p, retval = am_on_load; } - load_error: free_loader_state(magic); return retval; } @@ -1026,9 +1042,9 @@ loader_state_dtor(Binary* magic) } static Eterm -insert_new_code(Process *c_p, ErtsProcLocks c_p_locks, - Eterm group_leader, Eterm module, BeamCodeHeader* code_hdr, - Uint size) +stub_insert_new_code(Process *c_p, ErtsProcLocks c_p_locks, + Eterm group_leader, Eterm module, + BeamCodeHeader* code_hdr, Uint size) { Module* modp; Eterm retval; @@ -4700,14 +4716,13 @@ freeze_code(LoaderState* stp) } static void -final_touch(LoaderState* stp) +final_touch(LoaderState* stp, struct erl_module_instance* inst_p) { int i; int on_load = stp->on_load; unsigned catches; Uint index; BeamInstr* codev = stp->codev; - Module* modp; /* * Allocate catch indices and fix up all catch_yf instructions. @@ -4722,8 +4737,7 @@ final_touch(LoaderState* stp) codev[index+2] = make_catch(catches); index = next; } - modp = erts_put_module(stp->module); - modp->curr.catches = catches; + inst_p->catches = catches; /* * Export functions. @@ -6421,7 +6435,8 @@ erts_make_stub_module(Process* p, Eterm Mod, Eterm Beam, Eterm Info) * Insert the module in the module table. */ - rval = insert_new_code(p, 0, p->group_leader, Mod, code_hdr, code_size); + rval = stub_insert_new_code(p, 0, p->group_leader, Mod, + code_hdr, code_size); if (rval != NIL) { goto error; } -- cgit v1.2.3 From f77ea1c049c585db93618991eb02d9afbfa7ad21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 12 Apr 2016 07:14:57 +0200 Subject: Reimplement -on_load() Load the module as old code; swap old and new code if the -on_load function succeeds. That way, a failed update attempt for a module that has an -on_load function will preserve the previous version of the code. --- erts/emulator/beam/beam_bif_load.c | 63 +++++++++++++++----------- erts/emulator/beam/beam_load.c | 58 ++++++++++++++++++++---- erts/emulator/beam/erl_nif.c | 62 +++++++++++++++----------- lib/kernel/src/code_server.erl | 1 + lib/kernel/test/code_SUITE.erl | 91 +++++++++++++++++++++++++++++++++++++- 5 files changed, 212 insertions(+), 63 deletions(-) diff --git a/erts/emulator/beam/beam_bif_load.c b/erts/emulator/beam/beam_bif_load.c index 87508dcf5f..b6f5d66166 100644 --- a/erts/emulator/beam/beam_bif_load.c +++ b/erts/emulator/beam/beam_bif_load.c @@ -625,8 +625,8 @@ BIF_RETTYPE call_on_load_function_1(BIF_ALIST_1) { Module* modp = erts_get_module(BIF_ARG_1, erts_active_code_ix()); - if (modp && modp->curr.code_hdr) { - BIF_TRAP_CODE_PTR_0(BIF_P, modp->curr.code_hdr->on_load_function_ptr); + if (modp && modp->old.code_hdr) { + BIF_TRAP_CODE_PTR_0(BIF_P, modp->old.code_hdr->on_load_function_ptr); } else { BIF_ERROR(BIF_P, BADARG); @@ -651,14 +651,14 @@ BIF_RETTYPE finish_after_on_load_2(BIF_ALIST_2) code_ix = erts_active_code_ix(); modp = erts_get_module(BIF_ARG_1, code_ix); - if (!modp || !modp->curr.code_hdr) { + if (!modp || !modp->old.code_hdr) { error: erts_smp_thr_progress_unblock(); erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN); erts_release_code_write_permission(); BIF_ERROR(BIF_P, BADARG); } - if (modp->curr.code_hdr->on_load_function_ptr == NULL) { + if (modp->old.code_hdr->on_load_function_ptr == NULL) { goto error; } if (BIF_ARG_2 != am_false && BIF_ARG_2 != am_true) { @@ -667,44 +667,55 @@ BIF_RETTYPE finish_after_on_load_2(BIF_ALIST_2) if (BIF_ARG_2 == am_true) { int i; + struct erl_module_instance t; + + /* + * Swap old and new code. + */ + t = modp->curr; + modp->curr = modp->old; + modp->old = t; /* * The on_load function succeded. Fix up export entries. */ for (i = 0; i < export_list_size(code_ix); i++) { Export *ep = export_list(i,code_ix); - if (ep != NULL && - ep->code[0] == BIF_ARG_1 && - ep->code[4] != 0) { + if (ep == NULL || ep->code[0] != BIF_ARG_1) { + continue; + } + if (ep->code[4] != 0) { ep->addressv[code_ix] = (void *) ep->code[4]; ep->code[4] = 0; + } else { + if (ep->addressv[code_ix] == ep->code+3 && + ep->code[3] == (BeamInstr) em_apply_bif) { + continue; + } + ep->addressv[code_ix] = ep->code+3; + ep->code[3] = (BeamInstr) em_call_error_handler; } } modp->curr.code_hdr->on_load_function_ptr = NULL; set_default_trace_pattern(BIF_ARG_1); } else if (BIF_ARG_2 == am_false) { - BeamInstr* code; - BeamInstr* end; + int i; /* - * The on_load function failed. Remove the loaded code. - * This is an combination of delete and purge. We purge - * the current code; the old code is not touched. + * The on_load function failed. Remove references to the + * code that is about to be purged from the export entries. */ - erts_total_code_size -= modp->curr.code_length; - code = (BeamInstr*) modp->curr.code_hdr; - end = (BeamInstr *) ((char *)code + modp->curr.code_length); - erts_cleanup_funs_on_purge(code, end); - beam_catches_delmod(modp->curr.catches, code, modp->curr.code_length, - erts_active_code_ix()); - if (modp->curr.code_hdr->literals_start) { - erts_free(ERTS_ALC_T_LITERAL, modp->curr.code_hdr->literals_start); - } - erts_free(ERTS_ALC_T_CODE, modp->curr.code_hdr); - modp->curr.code_hdr = NULL; - modp->curr.code_length = 0; - modp->curr.catches = BEAM_CATCHES_NIL; - erts_remove_from_ranges(code); + + for (i = 0; i < export_list_size(code_ix); i++) { + Export *ep = export_list(i,code_ix); + if (ep == NULL || ep->code[0] != BIF_ARG_1) { + continue; + } + if (ep->code[3] == (BeamInstr) em_apply_bif) { + continue; + } + ep->code[4] = 0; + } } erts_smp_thr_progress_unblock(); erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN); diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index 9bd9b4bfc0..95489d9a63 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -32,6 +32,7 @@ #include "bif.h" #include "external.h" #include "beam_load.h" +#include "beam_bp.h" #include "big.h" #include "erl_bits.h" #include "beam_catches.h" @@ -766,7 +767,7 @@ Eterm erts_finish_loading(Binary* magic, Process* c_p, ErtsProcLocks c_p_locks, Eterm* modp) { - Eterm retval; + Eterm retval = NIL; LoaderState* stp = ERTS_MAGIC_BIN_DATA(magic); Module* mod_tab_p; struct erl_module_instance* inst_p; @@ -779,16 +780,52 @@ erts_finish_loading(Binary* magic, Process* c_p, ERTS_SMP_LC_ASSERT(erts_initialized == 0 || erts_has_code_write_permission() || erts_smp_thr_progress_is_blocking()); - /* * Make current code for the module old and insert the new code * as current. This will fail if there already exists old code * for the module. */ + mod_tab_p = erts_put_module(stp->module); CHKBLK(ERTS_ALC_T_CODE,stp->code); - retval = beam_make_current_old(c_p, c_p_locks, stp->module); - ASSERT(retval == NIL); + if (!stp->on_load) { + /* + * Normal case -- no -on_load() function. + */ + retval = beam_make_current_old(c_p, c_p_locks, stp->module); + ASSERT(retval == NIL); + } else { + ErtsCodeIndex code_ix = erts_staging_code_ix(); + Eterm module = stp->module; + int i; + + /* + * There is an -on_load() function. We will keep the current + * code, but we must turn off any tracing. + */ + + for (i = 0; i < export_list_size(code_ix); i++) { + Export *ep = export_list(i, code_ix); + if (ep == NULL || ep->code[0] != module) { + continue; + } + if (ep->addressv[code_ix] == ep->code+3) { + if (ep->code[3] == (BeamInstr) em_apply_bif) { + continue; + } else if (ep->code[3] == + (BeamInstr) BeamOp(op_i_generic_breakpoint)) { + ERTS_SMP_LC_ASSERT(erts_smp_thr_progress_is_blocking()); + ASSERT(mod_tab_p->curr.num_traced_exports > 0); + erts_clear_export_break(mod_tab_p, ep->code+3); + ep->addressv[code_ix] = (BeamInstr *) ep->code[4]; + ep->code[4] = 0; + } + ASSERT(ep->code[4] == 0); + } + } + ASSERT(mod_tab_p->curr.num_breakpoints == 0); + ASSERT(mod_tab_p->curr.num_traced_exports == 0); + } /* * Update module table. @@ -796,8 +833,11 @@ erts_finish_loading(Binary* magic, Process* c_p, size = stp->loaded_size; erts_total_code_size += size; - mod_tab_p = erts_put_module(stp->module); - inst_p = &mod_tab_p->curr; + if (stp->on_load) { + inst_p = &mod_tab_p->old; + } else { + inst_p = &mod_tab_p->curr; + } inst_p->code_hdr = stp->hdr; inst_p->code_length = size; @@ -4757,10 +4797,10 @@ final_touch(LoaderState* stp, struct erl_module_instance* inst_p) ep->addressv[erts_staging_code_ix()] = address; } else { /* - * Don't make any of the exported functions - * callable yet. + * on_load: Don't make any of the exported functions + * callable yet. Keep any function in the current + * code callable. */ - ep->addressv[erts_staging_code_ix()] = ep->code+3; ep->code[4] = (BeamInstr) address; } } diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c index 941f44b9ec..dc83a780a2 100644 --- a/erts/emulator/beam/erl_nif.c +++ b/erts/emulator/beam/erl_nif.c @@ -2780,7 +2780,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) ErlNifEntry* entry = NULL; ErlNifEnv env; int i, err, encoding; - Module* mod; + Module* module_p; Eterm mod_atom; const Atom* mod_atomp; Eterm f_atom; @@ -2790,6 +2790,8 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) int veto; struct erl_module_nif* lib = NULL; int reload_warning = 0; + struct erl_module_instance* this_mi; + struct erl_module_instance* prev_mi; encoding = erts_get_native_filename_encoding(); if (encoding == ERL_FILENAME_WIN_WCHAR) { @@ -2823,21 +2825,29 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) ASSERT(caller != NULL); mod_atom = caller[0]; ASSERT(is_atom(mod_atom)); - mod=erts_get_module(mod_atom, erts_active_code_ix()); - ASSERT(mod != NULL); + module_p = erts_get_module(mod_atom, erts_active_code_ix()); + ASSERT(module_p != NULL); mod_atomp = atom_tab(atom_val(mod_atom)); init_func = erts_static_nif_get_nif_init((char*)mod_atomp->name, mod_atomp->len); if (init_func != NULL) handle = init_func; - if (!in_area(caller, mod->curr.code_hdr, mod->curr.code_length)) { - ASSERT(in_area(caller, mod->old.code_hdr, mod->old.code_length)); + if (in_area(caller, module_p->old.code_hdr, module_p->old.code_length)) { + if (module_p->old.code_hdr->on_load_function_ptr) { + this_mi = &module_p->old; + prev_mi = &module_p->curr; + } else { + ret = load_nif_error(BIF_P, "old_code", "Calling load_nif from old " + "module '%T' not allowed", mod_atom); + goto error; + } + } else { + this_mi = &module_p->curr; + prev_mi = &module_p->old; + } - ret = load_nif_error(BIF_P, "old_code", "Calling load_nif from old " - "module '%T' not allowed", mod_atom); - } - else if (init_func == NULL && + if (init_func == NULL && (err=erts_sys_ddll_open(lib_name, &handle, &errdesc)) != ERL_DE_NO_ERROR) { const char slogan[] = "Failed to load NIF library"; if (strstr(errdesc.str, lib_name) != NULL) { @@ -2886,7 +2896,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) for (i=0; i < entry->num_of_funcs && ret==am_ok; i++) { BeamInstr** code_pp; if (!erts_atom_get(f->name, sys_strlen(f->name), &f_atom, ERTS_ATOM_ENC_LATIN1) - || (code_pp = get_func_pp(mod->curr.code_hdr, f_atom, f->arity))==NULL) { + || (code_pp = get_func_pp(this_mi->code_hdr, f_atom, f->arity))==NULL) { ret = load_nif_error(BIF_P,bad_lib,"Function not found %T:%s/%u", mod_atom, f->name, f->arity); } @@ -2937,9 +2947,9 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) erts_refc_init(&lib->rt_cnt, 0); erts_refc_init(&lib->rt_dtor_cnt, 0); ASSERT(opened_rt_list == NULL); - lib->mod = mod; + lib->mod = module_p; env.mod_nif = lib; - if (mod->curr.nif != NULL) { /*************** Reload ******************/ + if (this_mi->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 @@ -2947,16 +2957,16 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) */ int k, old_incr = 0; ErlNifFunc* old_func; - lib->priv_data = mod->curr.nif->priv_data; + lib->priv_data = this_mi->nif->priv_data; - ASSERT(mod->curr.nif->entry != NULL); + ASSERT(this_mi->nif->entry != NULL); if (entry->reload == NULL) { ret = load_nif_error(BIF_P,reload,"Reload not supported by this NIF library."); goto error; } /* Check that no NIF is removed */ - old_func = mod->curr.nif->entry->funcs; - for (k=0; k < mod->curr.nif->entry->num_of_funcs; k++) { + old_func = this_mi->nif->entry->funcs; + for (k=0; k < this_mi->nif->entry->num_of_funcs; k++) { int incr = 0; ErlNifFunc* f = entry->funcs; for (i=0; i < entry->num_of_funcs; i++) { @@ -2972,7 +2982,7 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) old_func->name, old_func->arity); goto error; } - old_func = next_func(mod->curr.nif->entry, &old_incr, old_func); + old_func = next_func(this_mi->nif->entry, &old_incr, old_func); } erts_pre_nif(&env, BIF_P, lib, NULL); veto = entry->reload(&env, &lib->priv_data, BIF_ARG_2); @@ -2982,24 +2992,24 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) } else { commit_opened_resource_types(lib); - mod->curr.nif->entry = NULL; /* to prevent 'unload' callback */ - erts_unload_nif(mod->curr.nif); + this_mi->nif->entry = NULL; /* to prevent 'unload' callback */ + erts_unload_nif(this_mi->nif); reload_warning = 1; } } else { lib->priv_data = NULL; - if (mod->old.nif != NULL) { /**************** Upgrade ***************/ - void* prev_old_data = mod->old.nif->priv_data; + if (prev_mi->nif != NULL) { /**************** Upgrade ***************/ + void* prev_old_data = prev_mi->nif->priv_data; if (entry->upgrade == NULL) { ret = load_nif_error(BIF_P, upgrade, "Upgrade not supported by this NIF library."); goto error; } - erts_pre_nif(&env, BIF_P, lib, NULL); - veto = entry->upgrade(&env, &lib->priv_data, &mod->old.nif->priv_data, BIF_ARG_2); + erts_pre_nif(&env, BIF_P, lib, NULL); + veto = entry->upgrade(&env, &lib->priv_data, &prev_mi->nif->priv_data, BIF_ARG_2); erts_post_nif(&env); if (veto) { - mod->old.nif->priv_data = prev_old_data; + prev_mi->nif->priv_data = prev_old_data; ret = load_nif_error(BIF_P, upgrade, "Library upgrade-call unsuccessful."); } else @@ -3024,12 +3034,12 @@ BIF_RETTYPE load_nif_2(BIF_ALIST_2) int incr = 0; ErlNifFunc* f = entry->funcs; - mod->curr.nif = lib; + this_mi->nif = lib; for (i=0; i < entry->num_of_funcs; i++) { BeamInstr* code_ptr; erts_atom_get(f->name, sys_strlen(f->name), &f_atom, ERTS_ATOM_ENC_LATIN1); - code_ptr = *get_func_pp(mod->curr.code_hdr, f_atom, f->arity); + code_ptr = *get_func_pp(this_mi->code_hdr, f_atom, f->arity); if (code_ptr[1] == 0) { code_ptr[5+0] = (BeamInstr) BeamOp(op_call_nif); diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl index 90b2a06c46..56372f90ca 100644 --- a/lib/kernel/src/code_server.erl +++ b/lib/kernel/src/code_server.erl @@ -1351,6 +1351,7 @@ finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db) -> Res = case Keep of false -> _ = finish_on_load_report(Mod, OnLoadRes), + _ = erts_code_purger:purge(Mod), {error,on_load_failure}; true -> ets:insert(Db, {Mod,File}), diff --git a/lib/kernel/test/code_SUITE.erl b/lib/kernel/test/code_SUITE.erl index a0b72925f7..547881f3b2 100644 --- a/lib/kernel/test/code_SUITE.erl +++ b/lib/kernel/test/code_SUITE.erl @@ -20,6 +20,7 @@ -module(code_SUITE). -include_lib("common_test/include/ct.hrl"). +-include_lib("syntax_tools/include/merl.hrl"). -export([all/0, suite/0,groups/0,init_per_group/2,end_per_group/2]). -export([set_path/1, get_path/1, add_path/1, add_paths/1, del_path/1, @@ -33,7 +34,9 @@ where_is_file/1, purge_stacktrace/1, mult_lib_roots/1, bad_erl_libs/1, code_archive/1, code_archive2/1, on_load/1, on_load_binary/1, - on_load_embedded/1, on_load_errors/1, big_boot_embedded/1, + on_load_embedded/1, on_load_errors/1, on_load_update/1, + on_load_purge/1, + big_boot_embedded/1, native_early_modules/1, get_mode/1, normalized_paths/1]). @@ -61,7 +64,8 @@ all() -> ext_mod_dep, clash, where_is_file, purge_stacktrace, mult_lib_roots, bad_erl_libs, code_archive, code_archive2, on_load, - on_load_binary, on_load_embedded, on_load_errors, + on_load_binary, on_load_embedded, on_load_errors, on_load_update, + on_load_purge, big_boot_embedded, native_early_modules, get_mode, normalized_paths]. groups() -> @@ -1436,6 +1440,89 @@ do_on_load_error(ReturnValue) -> {undef,[{on_load_error,main,[],_}|_]} = Exit end. +on_load_update(_Config) -> + {Mod,Code1} = on_load_update_code(1), + {module,Mod} = code:load_binary(Mod, "", Code1), + 42 = Mod:a(), + 100 = Mod:b(99), + 4 = erlang:trace_pattern({Mod,'_','_'}, true), + + {Mod,Code2} = on_load_update_code(2), + {error,on_load_failure} = code:load_binary(Mod, "", Code2), + 42 = Mod:a(), + 100 = Mod:b(99), + {'EXIT',{undef,_}} = (catch Mod:never()), + 4 = erlang:trace_pattern({Mod,'_','_'}, false), + + {Mod,Code3} = on_load_update_code(3), + {module,Mod} = code:load_binary(Mod, "", Code3), + 100 = Mod:c(), + {'EXIT',{undef,_}} = (catch Mod:a()), + {'EXIT',{undef,_}} = (catch Mod:b(10)), + {'EXIT',{undef,_}} = (catch Mod:never()), + + ok. + +on_load_update_code(Version) -> + Mod = ?FUNCTION_NAME, + Tree = on_load_update_code_1(Version, Mod), + io:format("Version ~p", [Version]), + {ok,Mod,Code} = merl:compile(Tree), + merl:print(Tree), + io:nl(), + {Mod,Code}. + +on_load_update_code_1(1, Mod) -> + ?Q(["-module('@Mod@').\n", + "-export([a/0,b/1]).\n" + "-on_load(f/0).\n", + "f() -> ok.\n", + "a() -> 42.\n" + "b(I) -> I+1.\n"]); +on_load_update_code_1(2, Mod) -> + ?Q(["-module('@Mod@').\n", + "-export([never/0]).\n" + "-on_load(f/0).\n", + "f() -> 42 = '@Mod@':a(), 1 = '@Mod@':b(0), fail.\n", + "never() -> never.\n"]); +on_load_update_code_1(3, Mod) -> + ?Q(["-module('@Mod@').\n", + "-export([c/0]).\n" + "-on_load(f/0).\n", + "f() -> ok.\n", + "c() -> 100.\n"]). + +on_load_purge(_Config) -> + Mod = ?FUNCTION_NAME, + register(Mod, self()), + Tree = ?Q(["-module('@Mod@').\n", + "-on_load(f/0).\n", + "loop() -> loop().\n", + "f() ->\n", + "'@Mod@' ! {self(),spawn(fun loop/0)},\n", + "receive Ack -> Ack end.\n"]), + merl:print(Tree), + {ok,Mod,Code} = merl:compile(Tree), + P = spawn(fun() -> + exit(code:load_binary(Mod, "", Code)) + end), + monitor(process, P), + receive + {Pid1,Pid2} -> + monitor(process, Pid2), + Pid1 ! ack_and_failure, + receive + {'DOWN',_,process,P,Exit1} -> + {error,on_load_failure} = Exit1 + end, + receive + {'DOWN',_,process,Pid2,Exit2} -> + io:format("~p\n", [Exit2]) + after 10000 -> + ct:fail(no_down_message) + end + end. + %% Test that the native code of early loaded modules is loaded. native_early_modules(Config) when is_list(Config) -> case erlang:system_info(hipe_architecture) of -- cgit v1.2.3 From eb7c5e3fa1f2d2bac21c4714b3a627a5517f797d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 3 May 2016 06:10:22 +0200 Subject: code_server: Eliminate unnecessary Tag in handle_call/3 For historical reasons, the second argument in handle_call/3 is {Pid,Tag}. The tag is always 'call' and is never used. --- lib/kernel/src/code_server.erl | 110 +++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 54 deletions(-) diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl index 56372f90ca..356c6630fa 100644 --- a/lib/kernel/src/code_server.erl +++ b/lib/kernel/src/code_server.erl @@ -142,7 +142,7 @@ reply(Pid, Res) -> loop(#state{supervisor=Supervisor}=State0) -> receive {code_call, Pid, Req} -> - case handle_call(Req, {Pid, call}, State0) of + case handle_call(Req, Pid, State0) of {reply, Res, State} -> _ = reply(Pid, Res), loop(State); @@ -225,90 +225,90 @@ system_code_change(State, _Module, _OldVsn, _Extra) -> %% The gen_server call back functions. %% -handle_call({stick_dir,Dir}, {_From,_Tag}, S) -> +handle_call({stick_dir,Dir}, _From, S) -> {reply,stick_dir(Dir, true, S),S}; -handle_call({unstick_dir,Dir}, {_From,_Tag}, S) -> +handle_call({unstick_dir,Dir}, _From, S) -> {reply,stick_dir(Dir, false, S),S}; -handle_call({stick_mod,Mod}, {_From,_Tag}, S) -> +handle_call({stick_mod,Mod}, _From, S) -> {reply,stick_mod(Mod, true, S),S}; -handle_call({unstick_mod,Mod}, {_From,_Tag}, S) -> +handle_call({unstick_mod,Mod}, _From, S) -> {reply,stick_mod(Mod, false, S),S}; -handle_call({dir,Dir}, {_From,_Tag}, S) -> +handle_call({dir,Dir}, _From, S) -> Root = S#state.root, Resp = do_dir(Root,Dir,S#state.namedb), {reply,Resp,S}; -handle_call({load_file,Mod}, Caller, St) when is_atom(Mod) -> - load_file(Mod, Caller, St); +handle_call({load_file,Mod}, From, St) when is_atom(Mod) -> + load_file(Mod, From, St); -handle_call({add_path,Where,Dir0}, {_From,_Tag}, +handle_call({add_path,Where,Dir0}, _From, #state{namedb=Namedb,path=Path0}=S) -> {Resp,Path} = add_path(Where, Dir0, Path0, Namedb), {reply,Resp,S#state{path=Path}}; -handle_call({add_paths,Where,Dirs0}, {_From,_Tag}, +handle_call({add_paths,Where,Dirs0}, _From, #state{namedb=Namedb,path=Path0}=S) -> {Resp,Path} = add_paths(Where, Dirs0, Path0, Namedb), {reply,Resp,S#state{path=Path}}; -handle_call({set_path,PathList}, {_From,_Tag}, +handle_call({set_path,PathList}, _From, #state{path=Path0,namedb=Namedb}=S) -> {Resp,Path,NewDb} = set_path(PathList, Path0, Namedb), {reply,Resp,S#state{path=Path,namedb=NewDb}}; -handle_call({del_path,Name}, {_From,_Tag}, +handle_call({del_path,Name}, _From, #state{path=Path0,namedb=Namedb}=S) -> {Resp,Path} = del_path(Name, Path0, Namedb), {reply,Resp,S#state{path=Path}}; -handle_call({replace_path,Name,Dir}, {_From,_Tag}, +handle_call({replace_path,Name,Dir}, _From, #state{path=Path0,namedb=Namedb}=S) -> {Resp,Path} = replace_path(Name, Dir, Path0, Namedb), {reply,Resp,S#state{path=Path}}; -handle_call(get_path, {_From,_Tag}, S) -> +handle_call(get_path, _From, S) -> {reply,S#state.path,S}; %% Messages to load, delete and purge modules/files. -handle_call({load_abs,File,Mod}, Caller, S) when is_atom(Mod) -> +handle_call({load_abs,File,Mod}, From, S) when is_atom(Mod) -> case modp(File) of false -> {reply,{error,badarg},S}; true -> - load_abs(File, Mod, Caller, S) + load_abs(File, Mod, From, S) end; -handle_call({load_binary,Mod,File,Bin}, Caller, S) when is_atom(Mod) -> - do_load_binary(Mod, File, Bin, Caller, S); +handle_call({load_binary,Mod,File,Bin}, From, S) when is_atom(Mod) -> + do_load_binary(Mod, File, Bin, From, S); -handle_call({load_native_partial,Mod,Bin}, {_From,_Tag}, S) -> +handle_call({load_native_partial,Mod,Bin}, _From, S) -> Architecture = erlang:system_info(hipe_architecture), Result = (catch hipe_unified_loader:load(Mod, Bin, Architecture)), Status = hipe_result_to_status(Result, S), {reply,Status,S}; -handle_call({load_native_sticky,Mod,Bin,WholeModule}, {_From,_Tag}, S) -> +handle_call({load_native_sticky,Mod,Bin,WholeModule}, _From, S) -> Architecture = erlang:system_info(hipe_architecture), Result = (catch hipe_unified_loader:load_module(Mod, Bin, WholeModule, Architecture)), Status = hipe_result_to_status(Result, S), {reply,Status,S}; -handle_call({ensure_loaded,Mod}, Caller, St) when is_atom(Mod) -> +handle_call({ensure_loaded,Mod}, From, St) when is_atom(Mod) -> case erlang:module_loaded(Mod) of true -> {reply,{module,Mod},St}; false when St#state.mode =:= interactive -> - load_file(Mod, Caller, St); + load_file(Mod, From, St); false -> {reply,{error,embedded},St} end; -handle_call({delete,Mod}, {_From,_Tag}, St) when is_atom(Mod) -> +handle_call({delete,Mod}, _From, St) when is_atom(Mod) -> case catch erlang:delete_module(Mod) of true -> ets:delete(St#state.moddb, Mod), @@ -317,48 +317,50 @@ handle_call({delete,Mod}, {_From,_Tag}, St) when is_atom(Mod) -> {reply,false,St} end; -handle_call({purge,Mod}, {_From,_Tag}, St) when is_atom(Mod) -> +handle_call({purge,Mod}, _From, St) when is_atom(Mod) -> {reply,do_purge(Mod),St}; -handle_call({soft_purge,Mod}, {_From,_Tag}, St) when is_atom(Mod) -> +handle_call({soft_purge,Mod}, _From, St) when is_atom(Mod) -> {reply,do_soft_purge(Mod),St}; -handle_call({is_loaded,Mod}, {_From,_Tag}, St) when is_atom(Mod) -> +handle_call({is_loaded,Mod}, _From, St) when is_atom(Mod) -> {reply,is_loaded(Mod, St#state.moddb),St}; -handle_call(all_loaded, {_From,_Tag}, S) -> +handle_call(all_loaded, _From, S) -> Db = S#state.moddb, {reply,all_loaded(Db),S}; -handle_call({get_object_code,Mod}, {_From,_Tag}, St) when is_atom(Mod) -> +handle_call({get_object_code,Mod}, _From, St) when is_atom(Mod) -> Path = St#state.path, case mod_to_bin(Path, Mod) of {_,Bin,FName} -> {reply,{Mod,Bin,FName},St}; Error -> {reply,Error,St} end; -handle_call({is_sticky, Mod}, {_From,_Tag}, S) -> +handle_call({is_sticky, Mod}, _From, S) -> Db = S#state.moddb, {reply, is_sticky(Mod,Db), S}; -handle_call(stop,{_From,_Tag}, S) -> +handle_call(stop,_From, S) -> {stop,normal,stopped,S}; -handle_call({set_primary_archive, File, ArchiveBin, FileInfo, ParserFun}, {_From,_Tag}, S=#state{mode=Mode}) -> - case erl_prim_loader:set_primary_archive(File, ArchiveBin, FileInfo, ParserFun) of +handle_call({set_primary_archive, File, ArchiveBin, FileInfo, ParserFun}, + _From, S=#state{mode=Mode}) -> + case erl_prim_loader:set_primary_archive(File, ArchiveBin, FileInfo, + ParserFun) of {ok, Files} -> {reply, {ok, Mode, Files}, S}; {error, _Reason} = Error -> {reply, Error, S} end; -handle_call(get_mode, {_From,_Tag}, S=#state{mode=Mode}) -> +handle_call(get_mode, _From, S=#state{mode=Mode}) -> {reply, Mode, S}; -handle_call({finish_loading,Prepared,EnsureLoaded}, {_,_}, S) -> +handle_call({finish_loading,Prepared,EnsureLoaded}, _From, S) -> {reply,finish_loading(Prepared, EnsureLoaded, S),S}; -handle_call(Other,{_From,_Tag}, S) -> +handle_call(Other,_From, S) -> error_msg(" ** Codeserver*** ignoring ~w~n ",[Other]), {noreply,S}. @@ -1054,14 +1056,14 @@ add_paths(Where,[Dir|Tail],Path,NameDb) -> add_paths(_,_,Path,_) -> {ok,Path}. -do_load_binary(Module, File, Binary, Caller, St) -> +do_load_binary(Module, File, Binary, From, St) -> case modp(File) andalso is_binary(Binary) of true -> case erlang:module_loaded(Module) of true -> do_purge(Module); false -> ok end, - try_load_module(File, Module, Binary, Caller, St); + try_load_module(File, Module, Binary, From, St); false -> {reply,{error,badarg},St} end. @@ -1070,51 +1072,51 @@ modp(Atom) when is_atom(Atom) -> true; modp(List) when is_list(List) -> int_list(List); modp(_) -> false. -load_abs(File, Mod, Caller, St) -> +load_abs(File, Mod, From, St) -> Ext = objfile_extension(), FileName0 = lists:concat([File, Ext]), FileName = absname(FileName0), case erl_prim_loader:get_file(FileName) of {ok,Bin,_} -> - try_load_module(FileName, Mod, Bin, Caller, St); + try_load_module(FileName, Mod, Bin, From, St); error -> {reply,{error,nofile},St} end. -try_load_module(File, Mod, Bin, {From,_}=Caller, St0) -> +try_load_module(File, Mod, Bin, From, St0) -> case pending_on_load(Mod, From, St0) of no -> - try_load_module_1(File, Mod, Bin, Caller, St0); + try_load_module_1(File, Mod, Bin, From, St0); {yes,St} -> {noreply,St} end. -try_load_module_1(File, Mod, Bin, Caller, #state{moddb=Db}=St) -> +try_load_module_1(File, Mod, Bin, From, #state{moddb=Db}=St) -> case is_sticky(Mod, Db) of true -> %% Sticky file reject the load error_msg("Can't load module '~w' that resides in sticky dir\n",[Mod]), {reply,{error,sticky_directory},St}; false -> Architecture = erlang:system_info(hipe_architecture), - try_load_module_2(File, Mod, Bin, Caller, Architecture, St) + try_load_module_2(File, Mod, Bin, From, Architecture, St) end. -try_load_module_2(File, Mod, Bin, Caller, undefined, St) -> - try_load_module_3(File, Mod, Bin, Caller, undefined, St); -try_load_module_2(File, Mod, Bin, Caller, Architecture, +try_load_module_2(File, Mod, Bin, From, undefined, St) -> + try_load_module_3(File, Mod, Bin, From, undefined, St); +try_load_module_2(File, Mod, Bin, From, Architecture, #state{moddb=Db}=St) -> case catch hipe_unified_loader:load_native_code(Mod, Bin, Architecture) of {module,Mod} = Module -> ets:insert(Db, [{{native,Mod},true},{Mod,File}]), {reply,Module,St}; no_native -> - try_load_module_3(File, Mod, Bin, Caller, Architecture, St); + try_load_module_3(File, Mod, Bin, From, Architecture, St); Error -> error_msg("Native loading of ~ts failed: ~p\n", [File,Error]), {reply,ok,St} end. -try_load_module_3(File, Mod, Bin, Caller, Architecture, +try_load_module_3(File, Mod, Bin, From, Architecture, #state{moddb=Db}=St) -> case erlang:load_module(Mod, Bin) of {module,Mod} = Module -> @@ -1122,7 +1124,7 @@ try_load_module_3(File, Mod, Bin, Caller, Architecture, post_beam_load([Mod], Architecture, St), {reply,Module,St}; {error,on_load} -> - handle_on_load(Mod, File, Caller, St); + handle_on_load(Mod, File, From, St); {error,What} = Error -> error_msg("Loading of ~ts failed: ~p\n", [File, What]), {reply,Error,St} @@ -1151,18 +1153,18 @@ int_list([H|T]) when is_integer(H) -> int_list(T); int_list([_|_]) -> false; int_list([]) -> true. -load_file(Mod, {From,_}=Caller, St0) -> +load_file(Mod, From, St0) -> case pending_on_load(Mod, From, St0) of - no -> load_file_1(Mod, Caller, St0); + no -> load_file_1(Mod, From, St0); {yes,St} -> {noreply,St} end. -load_file_1(Mod, Caller, #state{path=Path}=St) -> +load_file_1(Mod, From, #state{path=Path}=St) -> case mod_to_bin(Path, Mod) of error -> {reply,{error,nofile},St}; {Mod,Binary,File} -> - try_load_module_1(File, Mod, Binary, Caller, St) + try_load_module_1(File, Mod, Binary, From, St) end. mod_to_bin([Dir|Tail], Mod) -> @@ -1305,7 +1307,7 @@ run([F|Fs], Data0) -> %% The on_load functionality. %% ------------------------------------------------------- -handle_on_load(Mod, File, {From,_}, #state{on_load=OnLoad0}=St0) -> +handle_on_load(Mod, File, From, #state{on_load=OnLoad0}=St0) -> Fun = fun() -> Res = erlang:call_on_load_function(Mod), exit(Res) -- cgit v1.2.3 From f8e4ac533d388b39d5980092e5dd7a9d4ffee60b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Fri, 29 Apr 2016 15:50:19 +0200 Subject: Avoid deadlock when an on_load function makes an external call to the module itself --- lib/kernel/src/code_server.erl | 35 ++++++++++++++++++++++------------- lib/kernel/test/code_SUITE.erl | 30 ++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl index 356c6630fa..10264382eb 100644 --- a/lib/kernel/src/code_server.erl +++ b/lib/kernel/src/code_server.erl @@ -32,7 +32,8 @@ -import(lists, [foreach/2]). --type on_load_item() :: {reference(),module(),file:name_all(),[pid()]}. +-type on_load_item() :: {{pid(),reference()},module(), + file:name_all(),[pid()]}. -record(state, {supervisor :: pid(), root :: file:name_all(), @@ -155,8 +156,8 @@ loop(#state{supervisor=Supervisor}=State0) -> system_terminate(Reason, Supervisor, [], State0); {system, From, Msg} -> handle_system_msg(running,Msg, From, Supervisor, State0); - {'DOWN',Ref,process,_,Res} -> - State = finish_on_load(Ref, Res, State0), + {'DOWN',Ref,process,Pid,Res} -> + State = finish_on_load({Pid,Ref}, Res, State0), loop(State); _Msg -> loop(State0) @@ -1312,38 +1313,46 @@ handle_on_load(Mod, File, From, #state{on_load=OnLoad0}=St0) -> Res = erlang:call_on_load_function(Mod), exit(Res) end, - {_,Ref} = spawn_monitor(Fun), - OnLoad = [{Ref,Mod,File,[From]}|OnLoad0], + PidRef = spawn_monitor(Fun), + OnLoad = [{PidRef,Mod,File,[From]}|OnLoad0], St = St0#state{on_load=OnLoad}, {noreply,St}. pending_on_load(_, _, #state{on_load=[]}) -> no; pending_on_load(Mod, From, #state{on_load=OnLoad0}=St) -> - case lists:keymember(Mod, 2, OnLoad0) of + case lists:keyfind(Mod, 2, OnLoad0) of false -> no; - true -> + {{From,_Ref},Mod,_File,_Pids} -> + %% The on_load function tried to make an external + %% call to its own module. That would be a deadlock. + %% Fail the call. (The call is probably from error_handler, + %% and it will ignore the actual error reason and cause + %% an undef execption.) + _ = reply(From, {error,deadlock}), + {yes,St}; + {_,_,_,_} -> OnLoad = pending_on_load_1(Mod, From, OnLoad0), {yes,St#state{on_load=OnLoad}} end. -pending_on_load_1(Mod, From, [{Ref,Mod,File,Pids}|T]) -> - [{Ref,Mod,File,[From|Pids]}|T]; +pending_on_load_1(Mod, From, [{PidRef,Mod,File,Pids}|T]) -> + [{PidRef,Mod,File,[From|Pids]}|T]; pending_on_load_1(Mod, From, [H|T]) -> [H|pending_on_load_1(Mod, From, T)]; pending_on_load_1(_, _, []) -> []. -finish_on_load(Ref, OnLoadRes, #state{on_load=OnLoad0,moddb=Db}=State) -> - case lists:keyfind(Ref, 1, OnLoad0) of +finish_on_load(PidRef, OnLoadRes, #state{on_load=OnLoad0,moddb=Db}=State) -> + case lists:keyfind(PidRef, 1, OnLoad0) of false -> %% Since this process in general silently ignores messages %% it doesn't understand, it should also ignore a 'DOWN' %% message with an unknown reference. State; - {Ref,Mod,File,WaitingPids} -> + {PidRef,Mod,File,WaitingPids} -> finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db), - OnLoad = [E || {R,_,_,_}=E <- OnLoad0, R =/= Ref], + OnLoad = [E || {R,_,_,_}=E <- OnLoad0, R =/= PidRef], State#state{on_load=OnLoad} end. diff --git a/lib/kernel/test/code_SUITE.erl b/lib/kernel/test/code_SUITE.erl index 547881f3b2..e238eea2de 100644 --- a/lib/kernel/test/code_SUITE.erl +++ b/lib/kernel/test/code_SUITE.erl @@ -35,7 +35,7 @@ purge_stacktrace/1, mult_lib_roots/1, bad_erl_libs/1, code_archive/1, code_archive2/1, on_load/1, on_load_binary/1, on_load_embedded/1, on_load_errors/1, on_load_update/1, - on_load_purge/1, + on_load_purge/1, on_load_self_call/1, big_boot_embedded/1, native_early_modules/1, get_mode/1, normalized_paths/1]). @@ -65,7 +65,7 @@ all() -> purge_stacktrace, mult_lib_roots, bad_erl_libs, code_archive, code_archive2, on_load, on_load_binary, on_load_embedded, on_load_errors, on_load_update, - on_load_purge, + on_load_purge, on_load_self_call, big_boot_embedded, native_early_modules, get_mode, normalized_paths]. groups() -> @@ -1523,6 +1523,32 @@ on_load_purge(_Config) -> end end. +on_load_self_call(_Config) -> + Mod = ?FUNCTION_NAME, + register(Mod, self()), + Tree = ?Q(["-module('@Mod@').\n", + "-export([ext/0]).\n", + "-on_load(f/0).\n", + "f() ->\n", + " '@Mod@' ! (catch '@Mod@':ext()),\n", + " ok.\n", + "ext() -> good_work.\n"]), + merl:print(Tree), + {ok,Mod,Code} = merl:compile(Tree), + + {'EXIT',{undef,_}} = on_load_do_load(Mod, Code), + good_work = on_load_do_load(Mod, Code), + + ok. + +on_load_do_load(Mod, Code) -> + spawn(fun() -> + {module,Mod} = code:load_binary(Mod, "", Code) + end), + receive + Any -> Any + end. + %% Test that the native code of early loaded modules is loaded. native_early_modules(Config) when is_list(Config) -> case erlang:system_info(hipe_architecture) of -- cgit v1.2.3 From 7a1ff887383674e786a7c22aceefb0365ed91818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 3 May 2016 06:51:25 +0200 Subject: Correctly handle multiple load attempts when on_load is pending If an on_load function had not yet returned, the code server would only correctly handle calls to code:ensure_loaded/1. That is, each process that called code:ensured_loaded/1 for the module in question would be suspended until the on_load function had returned. The code server handled calls to code:load_binary/1, code:load_file/1, and code:load_abs/1 in the same way as for code:ensure_loaded/1. That means that if call to one those functions attempted to load *different* code for the module, that code would not get loaded. Note that code:finish_loading/1 (code:atomic_load/1) will still return {error,pending_on_load} if there is a pending on_load function for one of the modules that are about to be loaded. The reason is that code:finish_loading/1 is meant to either succeed directly, or fail quickly if there is any problem. --- lib/kernel/src/code_server.erl | 131 ++++++++++++++++++++++++----------------- lib/kernel/test/code_SUITE.erl | 58 +++++++++++++++++- 2 files changed, 133 insertions(+), 56 deletions(-) diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl index 10264382eb..6174136507 100644 --- a/lib/kernel/src/code_server.erl +++ b/lib/kernel/src/code_server.erl @@ -32,8 +32,12 @@ -import(lists, [foreach/2]). +-type on_load_action() :: + fun((term(), state()) -> {'reply',term(),state()} | + {'noreply',state()}). + -type on_load_item() :: {{pid(),reference()},module(), - file:name_all(),[pid()]}. + [{pid(),on_load_action()}]}. -record(state, {supervisor :: pid(), root :: file:name_all(), @@ -304,7 +308,7 @@ handle_call({ensure_loaded,Mod}, From, St) when is_atom(Mod) -> true -> {reply,{module,Mod},St}; false when St#state.mode =:= interactive -> - load_file(Mod, From, St); + ensure_loaded(Mod, From, St); false -> {reply,{error,embedded},St} end; @@ -1084,13 +1088,11 @@ load_abs(File, Mod, From, St) -> {reply,{error,nofile},St} end. -try_load_module(File, Mod, Bin, From, St0) -> - case pending_on_load(Mod, From, St0) of - no -> - try_load_module_1(File, Mod, Bin, From, St0); - {yes,St} -> - {noreply,St} - end. +try_load_module(File, Mod, Bin, From, St) -> + Action = fun(_, S) -> + try_load_module_1(File, Mod, Bin, From, S) + end, + handle_pending_on_load(Action, Mod, From, St). try_load_module_1(File, Mod, Bin, From, #state{moddb=Db}=St) -> case is_sticky(Mod, Db) of @@ -1117,19 +1119,19 @@ try_load_module_2(File, Mod, Bin, From, Architecture, {reply,ok,St} end. -try_load_module_3(File, Mod, Bin, From, Architecture, - #state{moddb=Db}=St) -> - case erlang:load_module(Mod, Bin) of - {module,Mod} = Module -> - ets:insert(Db, {Mod,File}), - post_beam_load([Mod], Architecture, St), - {reply,Module,St}; - {error,on_load} -> - handle_on_load(Mod, File, From, St); - {error,What} = Error -> - error_msg("Loading of ~ts failed: ~p\n", [File, What]), - {reply,Error,St} - end. +try_load_module_3(File, Mod, Bin, From, Architecture, St0) -> + Action = fun({module,_}=Module, #state{moddb=Db}=S) -> + ets:insert(Db, {Mod,File}), + post_beam_load([Mod], Architecture, S), + {reply,Module,S}; + ({error,on_load_failure}=Error, S) -> + {reply,Error,S}; + ({error,What}=Error, S) -> + error_msg("Loading of ~ts failed: ~p\n", [File, What]), + {reply,Error,S} + end, + Res = erlang:load_module(Mod, Bin), + handle_on_load(Res, Action, Mod, From, St0). hipe_result_to_status(Result, #state{moddb=Db}) -> case Result of @@ -1154,11 +1156,22 @@ int_list([H|T]) when is_integer(H) -> int_list(T); int_list([_|_]) -> false; int_list([]) -> true. +ensure_loaded(Mod, From, St0) -> + Action = fun(_, S) -> + case erlang:module_loaded(Mod) of + true -> + {reply,{module,Mod},S}; + false -> + load_file_1(Mod, From, S) + end + end, + handle_pending_on_load(Action, Mod, From, St0). + load_file(Mod, From, St0) -> - case pending_on_load(Mod, From, St0) of - no -> load_file_1(Mod, From, St0); - {yes,St} -> {noreply,St} - end. + Action = fun(_, S) -> + load_file_1(Mod, From, S) + end, + handle_pending_on_load(Action, Mod, From, St0). load_file_1(Mod, From, #state{path=Path}=St) -> case mod_to_bin(Path, Mod) of @@ -1308,55 +1321,56 @@ run([F|Fs], Data0) -> %% The on_load functionality. %% ------------------------------------------------------- -handle_on_load(Mod, File, From, #state{on_load=OnLoad0}=St0) -> +handle_on_load({error,on_load}, Action, Mod, From, St0) -> + #state{on_load=OnLoad0} = St0, Fun = fun() -> Res = erlang:call_on_load_function(Mod), exit(Res) end, PidRef = spawn_monitor(Fun), - OnLoad = [{PidRef,Mod,File,[From]}|OnLoad0], + PidAction = {From,Action}, + OnLoad = [{PidRef,Mod,[PidAction]}|OnLoad0], St = St0#state{on_load=OnLoad}, - {noreply,St}. + {noreply,St}; +handle_on_load(Res, Action, _, _, St) -> + Action(Res, St). -pending_on_load(_, _, #state{on_load=[]}) -> - no; -pending_on_load(Mod, From, #state{on_load=OnLoad0}=St) -> +handle_pending_on_load(Action, Mod, From, #state{on_load=OnLoad0}=St) -> case lists:keyfind(Mod, 2, OnLoad0) of false -> - no; - {{From,_Ref},Mod,_File,_Pids} -> + Action(ok, St); + {{From,_Ref},Mod,_Pids} -> %% The on_load function tried to make an external %% call to its own module. That would be a deadlock. %% Fail the call. (The call is probably from error_handler, %% and it will ignore the actual error reason and cause %% an undef execption.) - _ = reply(From, {error,deadlock}), - {yes,St}; - {_,_,_,_} -> - OnLoad = pending_on_load_1(Mod, From, OnLoad0), - {yes,St#state{on_load=OnLoad}} + {reply,{error,deadlock},St}; + {_,_,_} -> + OnLoad = handle_pending_on_load_1(Mod, {From,Action}, OnLoad0), + {noreply,St#state{on_load=OnLoad}} end. -pending_on_load_1(Mod, From, [{PidRef,Mod,File,Pids}|T]) -> - [{PidRef,Mod,File,[From|Pids]}|T]; -pending_on_load_1(Mod, From, [H|T]) -> - [H|pending_on_load_1(Mod, From, T)]; -pending_on_load_1(_, _, []) -> []. +handle_pending_on_load_1(Mod, From, [{PidRef,Mod,Pids}|T]) -> + [{PidRef,Mod,[From|Pids]}|T]; +handle_pending_on_load_1(Mod, From, [H|T]) -> + [H|handle_pending_on_load_1(Mod, From, T)]; +handle_pending_on_load_1(_, _, []) -> []. -finish_on_load(PidRef, OnLoadRes, #state{on_load=OnLoad0,moddb=Db}=State) -> +finish_on_load(PidRef, OnLoadRes, #state{on_load=OnLoad0}=St0) -> case lists:keyfind(PidRef, 1, OnLoad0) of false -> %% Since this process in general silently ignores messages %% it doesn't understand, it should also ignore a 'DOWN' %% message with an unknown reference. - State; - {PidRef,Mod,File,WaitingPids} -> - finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db), - OnLoad = [E || {R,_,_,_}=E <- OnLoad0, R =/= PidRef], - State#state{on_load=OnLoad} + St0; + {PidRef,Mod,Waiting} -> + St = finish_on_load_1(Mod, OnLoadRes, Waiting, St0), + OnLoad = [E || {R,_,_}=E <- OnLoad0, R =/= PidRef], + St#state{on_load=OnLoad} end. -finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db) -> +finish_on_load_1(Mod, OnLoadRes, Waiting, St) -> Keep = OnLoadRes =:= ok, erlang:finish_after_on_load(Mod, Keep), Res = case Keep of @@ -1365,11 +1379,20 @@ finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db) -> _ = erts_code_purger:purge(Mod), {error,on_load_failure}; true -> - ets:insert(Db, {Mod,File}), {module,Mod} end, - _ = [reply(Pid, Res) || Pid <- WaitingPids], - ok. + finish_on_load_2(Waiting, Res, St). + +finish_on_load_2([{Pid,Action}|T], Res, St0) -> + case Action(Res, St0) of + {reply,Rep,St} -> + _ = reply(Pid, Rep), + finish_on_load_2(T, Res, St); + {noreply,St} -> + finish_on_load_2(T, Res, St) + end; +finish_on_load_2([], _, St) -> + St. finish_on_load_report(_Mod, Atom) when is_atom(Atom) -> %% No error reports for atoms. diff --git a/lib/kernel/test/code_SUITE.erl b/lib/kernel/test/code_SUITE.erl index e238eea2de..8da67c89f8 100644 --- a/lib/kernel/test/code_SUITE.erl +++ b/lib/kernel/test/code_SUITE.erl @@ -35,7 +35,7 @@ purge_stacktrace/1, mult_lib_roots/1, bad_erl_libs/1, code_archive/1, code_archive2/1, on_load/1, on_load_binary/1, on_load_embedded/1, on_load_errors/1, on_load_update/1, - on_load_purge/1, on_load_self_call/1, + on_load_purge/1, on_load_self_call/1, on_load_pending/1, big_boot_embedded/1, native_early_modules/1, get_mode/1, normalized_paths/1]). @@ -65,7 +65,7 @@ all() -> purge_stacktrace, mult_lib_roots, bad_erl_libs, code_archive, code_archive2, on_load, on_load_binary, on_load_embedded, on_load_errors, on_load_update, - on_load_purge, on_load_self_call, + on_load_purge, on_load_self_call, on_load_pending, big_boot_embedded, native_early_modules, get_mode, normalized_paths]. groups() -> @@ -830,6 +830,12 @@ check_funs({'$M_EXPR',warning_msg,2}, [{code_server,finish_on_load_report,2} | _]) -> 0; check_funs({'$M_EXPR','$F_EXPR',1}, [{code_server,run,2}|_]) -> 0; +check_funs({'$M_EXPR','$F_EXPR',2}, + [{code_server,handle_on_load,5}|_]) -> 0; +check_funs({'$M_EXPR','$F_EXPR',2}, + [{code_server,handle_pending_on_load,4}|_]) -> 0; +check_funs({'$M_EXPR','$F_EXPR',2}, + [{code_server,finish_on_load_2,3}|_]) -> 0; %% This is cheating! /raimo %% %% check_funs(This = {M,_,_}, Path) -> @@ -1549,6 +1555,54 @@ on_load_do_load(Mod, Code) -> Any -> Any end. +on_load_pending(_Config) -> + Mod = ?FUNCTION_NAME, + Tree1 = ?Q(["-module('@Mod@').\n", + "-on_load(f/0).\n", + "f() ->\n", + " register('@Mod@', self()),\n", + " receive _ -> ok end.\n"]), + merl:print(Tree1), + {ok,Mod,Code1} = merl:compile(Tree1), + + Tree2 = ?Q(["-module('@Mod@').\n", + "-export([t/0]).\n", + "t() -> ok.\n"]), + merl:print(Tree2), + {ok,Mod,Code2} = merl:compile(Tree2), + + Self = self(), + {_,Ref1} = + spawn_monitor(fun() -> + Self ! started1, + {module,Mod} = code:load_binary(Mod, "", Code1) + end), + receive started1 -> ok end, + timer:sleep(10), + {_,Ref2} = + spawn_monitor(fun() -> + Self ! started2, + {module,Mod} = code:load_binary(Mod, "", Code2), + ok = Mod:t() + end), + receive started2 -> ok end, + receive + Unexpected -> + ct:fail({unexpected,Unexpected}) + after 100 -> + ok + end, + Mod ! go, + receive + {'DOWN',Ref1,process,_,normal} -> ok + end, + receive + {'DOWN',Ref2,process,_,normal} -> ok + end, + ok = Mod:t(), + ok. + + %% Test that the native code of early loaded modules is loaded. native_early_modules(Config) when is_list(Config) -> case erlang:system_info(hipe_architecture) of -- cgit v1.2.3 From 72fc3ab8b5bd19cbd7c9d0b507d97a942d4177de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Wed, 4 May 2016 09:45:24 +0200 Subject: Update documentation regarding improvements --- system/doc/reference_manual/code_loading.xml | 55 +++++++++++++--------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/system/doc/reference_manual/code_loading.xml b/system/doc/reference_manual/code_loading.xml index e9da40e02f..f6fd2911fa 100644 --- a/system/doc/reference_manual/code_loading.xml +++ b/system/doc/reference_manual/code_loading.xml @@ -130,26 +130,6 @@ loop() -> Running a Function When a Module is Loaded - -

The on_load feature is to be considered experimental - as there are a number of known weak points in current semantics, - which therefore might change in future Erlang/OTP releases:

- -

Doing external call in on_load to the module itself - leads to deadlock.

 -

At module upgrade, other processes calling the module - get suspended waiting for on_load to finish. This can be very bad - for applications with demands on realtime characteristics.

 -

At module upgrade, no rollback is done if the - on_load function fails. - The system is left in a bad limbo state without any working - and reachable instance of the module.

 - -

The problems with module upgrade described above can be fixed in future - Erlang/OTP releases by changing the behaviour to not make the module reachable until - after the on_load function has successfully returned.

- -

The -on_load() directive names a function that is to be run automatically when a module is loaded.

Its syntax is as follows:

@@ -159,20 +139,35 @@ loop() ->

It is not necessary to export the function. It is called in a freshly spawned process (which terminates as soon as the function - returns). The function must return ok if the module is to - remain loaded and become callable, or any other value if the module - is to be unloaded. Generating an exception also causes the - module to be unloaded. If the return value is not an atom, - a warning error report is sent to the error logger.

- -

A process that calls any function in a module whose on_load - function has not yet returned, is suspended until the on_load - function has returned.

+ returns).

+ +

The function must return ok if the module is to + become the new current code for the module and become + callable.

+ +

Returning any other value or generating an exception + causes the new code to be unloaded. If the return value is not an + atom, a warning error report is sent to the error logger.

+ +

If there already is current code for the module, that code will + remain current and can be called until the on_load function + has returned. If the on_load function fails, the current + code (if any) will remain current. If there is no current code for + a module, any process that makes an external call to the module + before the on_load function has finished will be suspended + until the on_load function have finished.

+ + +

Before OTP 19, if the on_load function failed, any + previously current code would become old, essentially leaving + the system without any working and reachable instance of the + module. That problem has been eliminated in OTP 19.

+

In embedded mode, first all modules are loaded. Then all on_load functions are called. The system is terminated unless all of the on_load functions return - ok

. + ok.

Example:

-- cgit v1.2.3