From 0248726417f6c0c709a85c0db930220de5541b9d Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Fri, 27 Jan 2012 19:05:31 +0100 Subject: erts: Fallback to blocking upgrade when tracing is enabled --- erts/emulator/beam/beam_bif_load.c | 166 +++++++++++---------- erts/emulator/beam/erl_bif_trace.c | 5 +- erts/emulator/beam/global.h | 1 + erts/emulator/test/call_trace_SUITE.erl | 120 ++++++++++++++- .../test/call_trace_SUITE_data/my_upgrade_test.erl | 26 ++++ 5 files changed, 238 insertions(+), 80 deletions(-) create mode 100644 erts/emulator/test/call_trace_SUITE_data/my_upgrade_test.erl (limited to 'erts/emulator') diff --git a/erts/emulator/beam/beam_bif_load.c b/erts/emulator/beam/beam_bif_load.c index 6694f3ae87..8d41c7499d 100644 --- a/erts/emulator/beam/beam_bif_load.c +++ b/erts/emulator/beam/beam_bif_load.c @@ -37,7 +37,6 @@ static void set_default_trace_pattern(Eterm module, ErtsCodeIndex); static Eterm check_process_code(Process* rp, Module* modp); -static void ensure_no_breakpoints(Process *, ErtsProcLocks, Eterm module); static void delete_code(Module* modp); static int purge_module(Process*, int module); static void decrement_refc(BeamInstr* code); @@ -55,6 +54,7 @@ load_module_2(BIF_ALIST_2) Eterm res; byte* temp_alloc = NULL; struct LoaderState* stp; + int is_blocking = 0; if (is_not_atom(BIF_ARG_1)) { error: @@ -79,18 +79,32 @@ load_module_2(BIF_ALIST_2) BIF_RET(res); } - /*SVERK - * Stop all other processes and finish the loading of the module. - * - erts_smp_proc_unlock(BIF_P, ERTS_PROC_LOCK_MAIN); - erts_smp_thr_progress_block();*/ - - ensure_no_breakpoints(BIF_P, 0, BIF_ARG_1); - erts_lock_code_ix(); - erts_start_staging_code_ix(); + for(;;) { + Module* modp; + erts_start_staging_code_ix(); + modp = erts_put_module(BIF_ARG_1); + if (!is_blocking) { + if (modp->curr.num_breakpoints > 0 + || modp->curr.num_traced_exports > 0 + || erts_is_default_trace_enabled()) { + /* we have tracing, retry single threaded */ + erts_abort_staging_code_ix(); + erts_unlock_code_ix(); + erts_smp_proc_unlock(BIF_P, ERTS_PROC_LOCK_MAIN); + erts_smp_thr_progress_block(); + is_blocking = 1; + continue; + } + } + else if (modp->curr.num_breakpoints) { + erts_clear_module_break(modp); + ASSERT(modp->curr.num_breakpoints == 0); + } + reason = erts_finish_loading(stp, BIF_P, 0, &BIF_ARG_1); + break; + } - reason = erts_finish_loading(stp, BIF_P, 0, &BIF_ARG_1); if (reason != NIL) { if (reason == am_on_load) { erts_commit_staging_code_ix(); @@ -105,11 +119,13 @@ load_module_2(BIF_ALIST_2) res = TUPLE2(hp, am_module, BIF_ARG_1); } - erts_unlock_code_ix(); - - /*SVERK - erts_smp_thr_progress_unblock(); - erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN);*/ + if (is_blocking) { + erts_smp_thr_progress_unblock(); + erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN); + } + else { + erts_unlock_code_ix(); + } BIF_RET(res); } @@ -162,17 +178,20 @@ BIF_RETTYPE code_is_module_native_1(BIF_ALIST_1) BIF_RETTYPE code_make_stub_module_3(BIF_ALIST_3) { + Module* modp; Eterm res; - /*SVERK erts_smp_proc_unlock(BIF_P, ERTS_PROC_LOCK_MAIN); erts_smp_thr_progress_block(); - erts_export_consolidate(erts_active_code_ix());*/ + modp = erts_get_module(BIF_ARG_1, erts_active_code_ix()); - ensure_no_breakpoints(BIF_P, 0, BIF_ARG_1); + if (modp && modp->curr.num_breakpoints > 0) { + ASSERT(modp->curr.code != NULL); + erts_clear_module_break(modp); + ASSERT(modp->curr.num_breakpoints == 0); + } - erts_lock_code_ix(); erts_start_staging_code_ix(); res = erts_make_stub_module(BIF_P, BIF_ARG_1, BIF_ARG_2, BIF_ARG_3); @@ -183,11 +202,8 @@ BIF_RETTYPE code_make_stub_module_3(BIF_ALIST_3) else { erts_abort_staging_code_ix(); } - erts_unlock_code_ix(); - - /*SVERK erts_smp_thr_progress_unblock(); - erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN);*/ + erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN); return res; } @@ -281,7 +297,9 @@ check_process_code_2(BIF_ALIST_2) BIF_RETTYPE delete_module_1(BIF_ALIST_1) { ErtsCodeIndex code_ix; - int res; + Module* modp; + int is_blocking = 0; + Eterm res = NIL; if (is_not_atom(BIF_ARG_1)) goto badarg; @@ -290,13 +308,11 @@ BIF_RETTYPE delete_module_1(BIF_ALIST_1) erts_smp_proc_unlock(BIF_P, ERTS_PROC_LOCK_MAIN); erts_smp_thr_progress_block();*/ - ensure_no_breakpoints(BIF_P, 0, BIF_ARG_1); - erts_lock_code_ix(); - erts_start_staging_code_ix(); - code_ix = erts_staging_code_ix(); - { - Module *modp = erts_get_module(BIF_ARG_1, code_ix); + do { + erts_start_staging_code_ix(); + code_ix = erts_staging_code_ix(); + modp = erts_get_module(BIF_ARG_1, code_ix); if (!modp) { res = am_undefined; } @@ -308,10 +324,26 @@ BIF_RETTYPE delete_module_1(BIF_ALIST_1) res = am_badarg; } else { + if (!is_blocking) { + if (modp->curr.num_breakpoints > 0 || + modp->curr.num_traced_exports > 0) { + /* we have tracing, retry single threaded */ + erts_abort_staging_code_ix(); + erts_unlock_code_ix(); + erts_smp_proc_unlock(BIF_P, ERTS_PROC_LOCK_MAIN); + erts_smp_thr_progress_block(); + is_blocking = 1; + continue; + } + } + else if (modp->curr.num_breakpoints) { + erts_clear_module_break(modp); + ASSERT(modp->curr.num_breakpoints == 0); + } delete_code(modp); res = am_true; } - } + } while (res == NIL); if (res == am_true) { erts_commit_staging_code_ix(); @@ -319,7 +351,13 @@ BIF_RETTYPE delete_module_1(BIF_ALIST_1) else { erts_abort_staging_code_ix(); } - erts_unlock_code_ix(); + if (is_blocking) { + erts_smp_thr_progress_unblock(); + erts_smp_proc_lock(BIF_P, ERTS_PROC_LOCK_MAIN); + } + else { + erts_unlock_code_ix(); + } /*SVERK erts_smp_thr_progress_unblock(); @@ -725,7 +763,7 @@ purge_module(Process* c_p, int module) BeamInstr* code; BeamInstr* end; Module* modp; - int is_blocked = 0; + int is_blocking = 0; int ret; erts_lock_code_ix(); @@ -753,13 +791,13 @@ retry: * Unload any NIF library */ if (modp->old.nif != NULL) { - if (!is_blocked) { + if (!is_blocking) { /*SVERK Do unload nif without blocking */ erts_rwunlock_old_code(code_ix); erts_unlock_code_ix(); erts_smp_proc_unlock(c_p, ERTS_PROC_LOCK_MAIN); erts_smp_thr_progress_block(); - is_blocked = 1; + is_blocking = 1; goto retry; } @@ -787,7 +825,7 @@ retry: } erts_rwunlock_old_code(code_ix); } - if (is_blocked) { + if (is_blocking) { erts_smp_thr_progress_unblock(); erts_smp_proc_lock(c_p, ERTS_PROC_LOCK_MAIN); } @@ -814,37 +852,6 @@ decrement_refc(BeamInstr* code) } } - -/* - * Clear breakpoints in module if any - */ -static void ensure_no_breakpoints(Process *c_p, ErtsProcLocks c_p_locks, - Eterm module) -{ - ErtsCodeIndex code_ix = erts_active_code_ix(); - Module* modp = erts_get_module(module, code_ix); - - if (modp && modp->curr.num_breakpoints > 0) { - ASSERT(modp->curr.code != NULL); -#ifdef ERTS_ENABLE_LOCK_CHECK -#ifdef ERTS_SMP - if (c_p && c_p_locks) - erts_proc_lc_chk_only_proc_main(c_p); - else -#endif - erts_lc_check_exact(NULL, 0); -#endif - if (c_p && c_p_locks) - erts_smp_proc_unlock(c_p, ERTS_PROC_LOCK_MAIN); - erts_smp_thr_progress_block(); - erts_clear_module_break(modp); - modp->curr.num_breakpoints = 0; - erts_smp_thr_progress_unblock(); - if (c_p && c_p_locks) - erts_smp_proc_lock(c_p, ERTS_PROC_LOCK_MAIN); - } -} - /* * Move code from current to old and null all export entries for the module */ @@ -859,19 +866,24 @@ delete_code(Module* modp) for (i = 0; i < export_list_size(code_ix); i++) { Export *ep = export_list(i, code_ix); if (ep != NULL && (ep->code[0] == module)) { - if (ep->addressv[code_ix] == ep->code+3 && - (ep->code[3] == (BeamInstr) em_apply_bif)) { - continue; + if (ep->addressv[code_ix] == ep->code+3) { + if (ep->code[3] == (BeamInstr) em_apply_bif) { + continue; + } + else if (ep->code[3] == (BeamInstr) em_call_traced_function) { + ERTS_SMP_LC_ASSERT(erts_smp_thr_progress_is_blocking()); + ASSERT(modp->curr.num_traced_exports > 0); + --modp->curr.num_traced_exports; + MatchSetUnref(ep->match_prog_set); + ep->match_prog_set = NULL; + } + else ASSERT(ep->code[3] == (BeamInstr) em_call_error_handler + || !erts_initialized); } ep->addressv[code_ix] = ep->code+3; - if (ep->code[3] == (BeamInstr) em_call_traced_function) { - ASSERT(modp->curr.num_traced_exports > 0); - --modp->curr.num_traced_exports; - } ep->code[3] = (BeamInstr) em_call_error_handler; ep->code[4] = 0; - MatchSetUnref(ep->match_prog_set); - ep->match_prog_set = NULL; + ASSERT(ep->match_prog_set == NULL); } } diff --git a/erts/emulator/beam/erl_bif_trace.c b/erts/emulator/beam/erl_bif_trace.c index 24b16b9679..203ba9004b 100644 --- a/erts/emulator/beam/erl_bif_trace.c +++ b/erts/emulator/beam/erl_bif_trace.c @@ -373,7 +373,10 @@ erts_get_default_trace_pattern(int *trace_pattern_is_on, *meta_tracer_pid = erts_default_meta_tracer_pid; } - +int erts_is_default_trace_enabled(void) +{ + return erts_default_trace_pattern_is_on; +} Uint erts_trace_flag2bit(Eterm flag) diff --git a/erts/emulator/beam/global.h b/erts/emulator/beam/global.h index b98f810455..580336470c 100644 --- a/erts/emulator/beam/global.h +++ b/erts/emulator/beam/global.h @@ -1773,6 +1773,7 @@ erts_get_default_trace_pattern(int *trace_pattern_is_on, Binary **meta_match_spec, struct trace_pattern_flags *trace_pattern_flags, Eterm *meta_tracer_pid); +int erts_is_default_trace_enabled(void); void erts_bif_trace_init(void); /* diff --git a/erts/emulator/test/call_trace_SUITE.erl b/erts/emulator/test/call_trace_SUITE.erl index 7030ebed3f..67212cd469 100644 --- a/erts/emulator/test/call_trace_SUITE.erl +++ b/erts/emulator/test/call_trace_SUITE.erl @@ -25,6 +25,7 @@ init_per_testcase/2,end_per_testcase/2, hipe/1,process_specs/1,basic/1,flags/1,errors/1,pam/1,change_pam/1, return_trace/1,exception_trace/1,on_load/1,deep_exception/1, + upgrade/1, exception_nocatch/1,bit_syntax/1]). %% Helper functions. @@ -46,6 +47,7 @@ suite() -> [{ct_hooks,[ts_install_cth]}]. all() -> Common = [errors, on_load], NotHipe = [process_specs, basic, flags, pam, change_pam, + upgrade, return_trace, exception_trace, deep_exception, exception_nocatch, bit_syntax], Hipe = [hipe], @@ -267,6 +269,118 @@ foo() -> foo0. foo(X) -> X+1. foo(X, Y) -> X+Y. + +%% Note that the semantics that this test case verifies +%% are not explicitly specified in the docs (what I could find in R15B). +%% This test case was written to verify that we do not change +%% any behaviour with the introduction of "block-free" upgrade in R16. +%% In short: Do not refer to this test case as an authority of how it must work. +upgrade(doc) -> + "Test tracing on module being upgraded"; +upgrade(Config) when is_list(Config) -> + V1 = compile_version(my_upgrade_test, 1, Config), + V2 = compile_version(my_upgrade_test, 2, Config), + start_tracer(), + upgrade_do(V1, V2, false), + upgrade_do(V1, V2, true). + +upgrade_do(V1, V2, TraceLocalVersion) -> + {module,my_upgrade_test} = erlang:load_module(my_upgrade_test, V1), + + + %% Test that trace is cleared after load_module + + trace_func({my_upgrade_test,'_','_'}, [], [global]), + case TraceLocalVersion of + true -> trace_func({my_upgrade_test,local_version,0}, [], [local]); + _ -> ok + end, + 1 = my_upgrade_test:version(), + 1 = my_upgrade_test:do_local(), + 1 = my_upgrade_test:do_real_local(), + put('F1_exp', my_upgrade_test:make_fun_exp()), + put('F1_loc', my_upgrade_test:make_fun_local()), + 1 = (get('F1_exp'))(), + 1 = (get('F1_loc'))(), + + Self = self(), + expect({trace,Self,call,{my_upgrade_test,version,[]}}), + expect({trace,Self,call,{my_upgrade_test,do_local,[]}}), + expect({trace,Self,call,{my_upgrade_test,do_real_local,[]}}), + case TraceLocalVersion of + true -> expect({trace,Self,call,{my_upgrade_test,local_version,[]}}); + _ -> ok + end, + expect({trace,Self,call,{my_upgrade_test,make_fun_exp,[]}}), + expect({trace,Self,call,{my_upgrade_test,make_fun_local,[]}}), + expect({trace,Self,call,{my_upgrade_test,version,[]}}), % F1_exp + case TraceLocalVersion of + true -> expect({trace,Self,call,{my_upgrade_test,local_version,[]}}); % F1_loc + _ -> ok + end, + + {module,my_upgrade_test} = erlang:load_module(my_upgrade_test, V2), + 2 = my_upgrade_test:version(), + put('F2_exp', my_upgrade_test:make_fun_exp()), + put('F2_loc', my_upgrade_test:make_fun_local()), + 2 = (get('F1_exp'))(), + 1 = (get('F1_loc'))(), + 2 = (get('F2_exp'))(), + 2 = (get('F2_loc'))(), + expect(), + + put('F1_exp', undefined), + put('F1_loc', undefined), + erlang:garbage_collect(), + erlang:purge_module(my_upgrade_test), + + % Test that trace is cleared after delete_module + + trace_func({my_upgrade_test,'_','_'}, [], [global]), + case TraceLocalVersion of + true -> trace_func({my_upgrade_test,local_version,0}, [], [local]); + _ -> ok + end, + 2 = my_upgrade_test:version(), + 2 = my_upgrade_test:do_local(), + 2 = my_upgrade_test:do_real_local(), + 2 = (get('F2_exp'))(), + 2 = (get('F2_loc'))(), + + expect({trace,Self,call,{my_upgrade_test,version,[]}}), + expect({trace,Self,call,{my_upgrade_test,do_local,[]}}), + expect({trace,Self,call,{my_upgrade_test,do_real_local,[]}}), + case TraceLocalVersion of + true -> expect({trace,Self,call,{my_upgrade_test,local_version,[]}}); + _ -> ok + end, + expect({trace,Self,call,{my_upgrade_test,version,[]}}), % F2_exp + case TraceLocalVersion of + true -> expect({trace,Self,call,{my_upgrade_test,local_version,[]}}); % F2_loc + _ -> ok + end, + + true = erlang:delete_module(my_upgrade_test), + {'EXIT',{undef,_}} = (catch my_upgrade_test:version()), + {'EXIT',{undef,_}} = (catch ((get('F2_exp'))())), + 2 = (get('F2_loc'))(), + expect(), + + put('F2_exp', undefined), + put('F2_loc', undefined), + erlang:garbage_collect(), + erlang:purge_module(my_upgrade_test), + ok. + +compile_version(Module, Version, Config) -> + Data = ?config(data_dir, Config), + File = filename:join(Data, atom_to_list(Module)), + {ok,Module,Bin} = compile:file(File, [{d,'VERSION',Version}, + binary,report]), + Bin. + + + %% Test flags (arity, timestamp) for call_trace/3. %% Also, test the '{tracer,Pid}' option. flags(_Config) -> @@ -1151,11 +1265,13 @@ trace_info(What, Key) -> Res. trace_func(MFA, MatchSpec) -> - get(tracer) ! {apply,self(),{erlang,trace_pattern,[MFA, MatchSpec]}}, + trace_func(MFA, MatchSpec, []). +trace_func(MFA, MatchSpec, Flags) -> + get(tracer) ! {apply,self(),{erlang,trace_pattern,[MFA, MatchSpec, Flags]}}, Res = receive {apply_result,Result} -> Result end, - ok = io:format("trace_pattern(~p, ~p) -> ~p", [MFA,MatchSpec,Res]), + ok = io:format("trace_pattern(~p, ~p, ~p) -> ~p", [MFA,MatchSpec,Flags,Res]), Res. trace_pid(Pid, On, Flags) -> diff --git a/erts/emulator/test/call_trace_SUITE_data/my_upgrade_test.erl b/erts/emulator/test/call_trace_SUITE_data/my_upgrade_test.erl new file mode 100644 index 0000000000..11b8a95209 --- /dev/null +++ b/erts/emulator/test/call_trace_SUITE_data/my_upgrade_test.erl @@ -0,0 +1,26 @@ +-module(my_upgrade_test). + +-export([version/0]). +-export([do_local/0]). +-export([do_real_local/0]). +-export([make_fun_exp/0]). +-export([make_fun_local/0]). + + +version() -> + ?VERSION. + +do_local() -> + version(). + +do_real_local() -> + local_version(). + +local_version() -> + ?VERSION. + +make_fun_exp() -> + fun() -> ?MODULE:version() end. + +make_fun_local() -> + fun() -> local_version() end. -- cgit v1.2.3