From 3692110ebd533773c47f3e9cab367c0207e739d4 Mon Sep 17 00:00:00 2001 From: Stavros Aronis Date: Tue, 20 Feb 2018 11:15:40 +0100 Subject: [PATCH] Remove untested code for hijacking processes This code was a failed attempt to support communication with processes not under Concuerror's control, e.g. application_controller. It has never become truly necessary. Closes #2. --- src/concuerror_callback.erl | 101 +++++------------- src/concuerror_inspect.erl | 23 +--- src/concuerror_scheduler.erl | 9 +- .../basic_tests/src/application_check.erl | 2 +- 4 files changed, 35 insertions(+), 100 deletions(-) diff --git a/src/concuerror_callback.erl b/src/concuerror_callback.erl index 56b60c8b5..1d5284f7b 100644 --- a/src/concuerror_callback.erl +++ b/src/concuerror_callback.erl @@ -3,7 +3,7 @@ -module(concuerror_callback). %% Interface to concuerror_inspect: --export([instrumented/4, hijack_backend/1]). +-export([instrumented/4]). %% Interface to scheduler: -export([spawn_first_process/1, start_first_process/3, @@ -106,7 +106,7 @@ %%------------------------------------------------------------------------------ --spec spawn_first_process(concuerror_options:options()) -> {pid(), [pid()]}. +-spec spawn_first_process(concuerror_options:options()) -> pid(). spawn_first_process(Options) -> EtsTables = ets:new(ets_tables, [public]), @@ -126,13 +126,13 @@ spawn_first_process(Options) -> timeout = ?opt(timeout, Options), timers = ets:new(timers, [public]) }, - System = system_processes_wrappers(Info), + system_processes_wrappers(Info), system_ets_entries(Info), P = new_process(Info), true = ets:insert(Processes, ?new_process(P, "P")), {DefLeader, _} = run_built_in(erlang, whereis, 1, [user], Info), true = ets:update_element(Processes, P, {?process_leader, DefLeader}), - {P, System}. + P. -spec start_first_process(pid(), {atom(), atom(), [term()]}, timeout()) -> ok. @@ -147,25 +147,12 @@ start_first_process(Pid, {Module, Name, Args}, Timeout) -> setup_logger(Processes) -> concuerror_inspect:start_inspection({logger, Processes}). --spec hijack_backend(concuerror_info()) -> ok. - -hijack_backend(#concuerror_info{processes = Processes} = Info) -> - true = group_leader() =:= whereis(user), - {GroupLeader, _} = run_built_in(erlang,whereis,1,[user], Info), - {registered_name, Name} = process_info(self(), registered_name), - true = ets:insert(Processes, ?new_system_process(self(), Name, hijacked)), - {true, Info} = - run_built_in(erlang, group_leader, 2, [GroupLeader, self()], Info), - concuerror_inspect:start_inspection(Info), - ok. - %%------------------------------------------------------------------------------ -type instrumented_return() :: 'doit' | {'didit', term()} | {'error', term()} | - {'skip_timeout', 'false' | {'true', term()}} | - 'unhijack'. + {'skip_timeout', 'false' | {'true', term()}}. -spec instrumented(Tag :: instrumented_tag(), Args :: [term()], @@ -862,15 +849,7 @@ run_built_in(erlang, unregister, 1, [Name], _:_ -> error(badarg) end; run_built_in(erlang, whereis, 1, [Name], Info) -> - #concuerror_info{ - logger = Logger, - processes = Processes - } = Info, - case Name of - application_controller -> - ?unique(Logger, ?lwarning, msg(whereis_application_controller), []); - _ -> ok - end, + #concuerror_info{processes = Processes} = Info, case ets:match(Processes, ?process_match_name_to_pid(Name)) of [] -> case whereis(Name) =:= undefined of @@ -1251,12 +1230,9 @@ enabled(P) -> handle_receive(PatternFun, Timeout, Location, Info) -> %% No distinction between replaying/new as we have to clear the message from %% the queue anyway... - case has_matching_or_after(PatternFun, Timeout, Location, Info, blocking) of - unhijack -> - {unhijack, Info}; - {MessageOrAfter, NewInfo} -> - handle_receive(MessageOrAfter, PatternFun, Timeout, Location, NewInfo) - end. + {MessageOrAfter, NewInfo} = + has_matching_or_after(PatternFun, Timeout, Location, Info, blocking), + handle_receive(MessageOrAfter, PatternFun, Timeout, Location, NewInfo). handle_receive(MessageOrAfter, PatternFun, Timeout, Location, Info) -> {Cnt, ReceiveInfo} = get_receive_cnt(Info), @@ -1289,8 +1265,6 @@ handle_receive(MessageOrAfter, PatternFun, Timeout, Location, Info) -> end, {{skip_timeout, AddMessage}, delay_notify(Notification, UpdatedInfo)}. -has_matching_or_after(_, _, _, unhijack, _) -> - unhijack; has_matching_or_after(PatternFun, Timeout, Location, InfoIn, Mode) -> {Result, NewOldMessages} = has_matching_or_after(PatternFun, Timeout, InfoIn), UpdatedInfo = update_messages(Result, NewOldMessages, InfoIn), @@ -1563,8 +1537,6 @@ process_loop(Info) -> {get_info, To} -> To ! {info, {Info, get()}}, process_loop(Info); - unhijack -> - unhijack; quit -> exit(normal) end. @@ -1786,13 +1758,9 @@ ets_ops_access_rights_map(Op) -> cleanup_processes(ProcessesTable) -> Processes = ets:tab2list(ProcessesTable), Foreach = - fun(?process_pat_pid_kind(P,Kind)) -> - case Kind =:= hijacked of - true -> P ! unhijack; - false -> - unlink(P), - P ! quit - end + fun(?process_pat_pid(P)) -> + unlink(P), + P ! quit end, lists:foreach(Foreach, Processes). @@ -1803,34 +1771,16 @@ system_ets_entries(#concuerror_info{ets_tables = EtsTables}) -> ets:insert(EtsTables, [Map(Tid) || Tid <- ets:all(), is_atom(Tid)]). system_processes_wrappers(Info) -> - Ordered = [user], - Registered = - Ordered ++ (lists:sort(registered() -- Ordered)), - [whereis(Name) || - Name <- Registered, - hijacked =:= hijack_or_wrap_system(Name, Info)]. - -%% XXX: Application controller support needs to be checked -hijack_or_wrap_system(Name, #concuerror_info{timeout = Timeout} = Info) - when Name =:= application_controller -> - Pid = whereis(Name), - link(Pid), - _ = concuerror_loader:load(gen_server), - ok = sys:suspend(Name), - Notify = - reset_concuerror_info( - Info#concuerror_info{notify_when_ready = {self(), true}}), - concuerror_inspect:hijack(Name, Notify), - ok = sys:resume(Name), - wait_process(Name, Timeout), - hijacked; -hijack_or_wrap_system(Name, Info) -> + [wrap_system(Name, Info) || Name <- registered()], + ok. + +wrap_system(Name, Info) -> #concuerror_info{processes = Processes} = Info, Wrapped = whereis(Name), Fun = fun() -> system_wrapper_loop(Name, Wrapped, Info) end, Pid = spawn_link(Fun), ets:insert(Processes, ?new_system_process(Pid, Name, wrapper)), - wrapped. + ok. system_wrapper_loop(Name, Wrapped, Info) -> receive @@ -1842,6 +1792,8 @@ system_wrapper_loop(Name, Wrapped, Info) -> try {F, R} = case Name of + application_controller -> + throw(comm_application_controller); code_server -> {Call, From, Request} = Data, check_request(Name, Request), @@ -2081,12 +2033,7 @@ msg(register_eunit_server) -> " one after another; as a result, systematic testing will have to" " explore a number of schedulings that is the product of every" " individual test's schedulings! You should use Concuerror on single tests" - " instead.~n"; -msg(whereis_application_controller) -> - "Your test communicates with the 'application_controller' process. This" - " can be problematic, as this process is not under Concuerror's" - " control. You may want to try to start the test from a top-level" - " supervisor (or even better a top level gen_server).~n". + " instead.~n". %%------------------------------------------------------------------------------ @@ -2099,6 +2046,14 @@ explain_error({checking_system_process, Pid}) -> " Concuerror." ?notify_us_msg, [Pid]); +explain_error(comm_application_controller) -> + io_lib:format( + "Your test communicates with the 'application_controller' process. This" + " is problematic, as this process is not under Concuerror's" + " control. Try to start the test from a top-level" + " supervisor (or even better a top level gen_server) instead.", + [] + ); explain_error({inconsistent_builtin, [Module, Name, Arity, Args, OldResult, NewResult, Location]}) -> io_lib:format( diff --git a/src/concuerror_inspect.erl b/src/concuerror_inspect.erl index 5b91415d8..e8b51e95e 100644 --- a/src/concuerror_inspect.erl +++ b/src/concuerror_inspect.erl @@ -4,7 +4,7 @@ -module(concuerror_inspect). %% Interface to instrumented code: --export([start_inspection/1, stop_inspection/0, inspect/3, hijack/2, explain_error/1]). +-export([start_inspection/1, stop_inspection/0, inspect/3, explain_error/1]). -include("concuerror.hrl"). @@ -38,14 +38,7 @@ stop_inspection() -> inspect(Tag, Args, Location) -> Ret = case stop_inspection() of - false -> - receive - {hijack, I} -> - concuerror_callback:hijack_backend(I), - retry - after - 0 -> doit - end; + false -> doit; {true, Info} -> {R, NewInfo} = concuerror_callback:instrumented(Tag, Args, Location, Info), @@ -64,17 +57,13 @@ inspect(Tag, Args, Location) -> end; {didit, Res} -> Res; {error, Reason} -> error(Reason); - retry -> inspect(Tag, Args, Location); {skip_timeout, CreateMessage} -> assert_no_messages(), case CreateMessage of false -> ok; {true, D} -> self() ! D end, - 0; - unhijack -> - erase(concuerror_info), - inspect(Tag, Args, Location) + 0 end. assert_no_messages() -> @@ -84,12 +73,6 @@ assert_no_messages() -> 0 -> ok end. --spec hijack(atom(), term()) -> ok. - -hijack(Name, Info) -> - Name ! {hijack, Info}, - ok. - -spec explain_error(term()) -> string(). explain_error({pending_message, Proc, Msg}) -> diff --git a/src/concuerror_scheduler.erl b/src/concuerror_scheduler.erl index c2fd2c750..95aee33f1 100644 --- a/src/concuerror_scheduler.erl +++ b/src/concuerror_scheduler.erl @@ -124,7 +124,6 @@ scheduling_bound_type :: concuerror_options:scheduling_bound_type(), show_races :: boolean(), strict_scheduling :: boolean(), - system :: [pid()], timeout :: timeout(), trace :: [trace_state()], treat_as_normal :: [atom()], @@ -142,8 +141,7 @@ run(Options) -> process_flag(trap_exit, true), put(bound_exceeded, false), - {FirstProcess, System} = - concuerror_callback:spawn_first_process(Options), + FirstProcess = concuerror_callback:spawn_first_process(Options), EntryPoint = ?opt(entry_point, Options), Timeout = ?opt(timeout, Options), ok = @@ -181,7 +179,6 @@ run(Options) -> scheduling_bound_type = SchedulingBoundType, show_races = ?opt(show_races, Options), strict_scheduling = ?opt(strict_scheduling, Options), - system = System, trace = [InitialTrace], treat_as_normal = ?opt(treat_as_normal, Options), timeout = Timeout, @@ -333,14 +330,14 @@ get_next_event(#scheduler_state{logger = _Logger, trace = [Last|_]} = State) -> get_next_event(Event, MaybeNeedsReplayState) -> State = replay(MaybeNeedsReplayState), - #scheduler_state{system = System, trace = [Last|_]} = State, + #scheduler_state{trace = [Last|_]} = State, #trace_state{actors = Actors, sleeping = Sleeping} = Last, SortedActors = schedule_sort(Actors, State), #event{actor = Actor, label = Label} = Event, case Actor =:= undefined of true -> AvailableActors = filter_sleeping(Sleeping, SortedActors), - free_schedule(Event, System ++ AvailableActors, State); + free_schedule(Event, AvailableActors, State); false -> #scheduler_state{print_depth = PrintDepth} = State, #trace_state{index = I} = Last, diff --git a/tests/suites/basic_tests/src/application_check.erl b/tests/suites/basic_tests/src/application_check.erl index c292703b1..677bf9740 100644 --- a/tests/suites/basic_tests/src/application_check.erl +++ b/tests/suites/basic_tests/src/application_check.erl @@ -9,7 +9,7 @@ %%------------------------------------------------------------------------------ -scenarios() -> [{test, inf, dpor}]. +scenarios() -> [{test, inf, dpor, crash}]. exceptional() -> fun(_Expected, Actual) ->