From 8ca53b0f993ffbf2991e3068b76ec15b8f5eca51 Mon Sep 17 00:00:00 2001 From: Zandra Norman Date: Mon, 23 Jan 2017 11:49:32 +0100 Subject: stdlib: Make gen_server callbacks optional --- lib/stdlib/doc/src/gen_server.xml | 18 +++ lib/stdlib/src/gen_server.erl | 42 +++-- lib/stdlib/test/erl_internal_SUITE.erl | 2 +- lib/stdlib/test/erl_lint_SUITE.erl | 10 +- lib/stdlib/test/gen_server_SUITE.erl | 175 ++++++++++++++++++++- .../test/gen_server_SUITE_data/oc_server.erl | 37 +++++ 6 files changed, 258 insertions(+), 26 deletions(-) create mode 100644 lib/stdlib/test/gen_server_SUITE_data/oc_server.erl (limited to 'lib/stdlib') diff --git a/lib/stdlib/doc/src/gen_server.xml b/lib/stdlib/doc/src/gen_server.xml index 662076b5f0..4540449792 100644 --- a/lib/stdlib/doc/src/gen_server.xml +++ b/lib/stdlib/doc/src/gen_server.xml @@ -504,6 +504,13 @@ gen_server:abcast -----> Module:handle_cast/2 Reason = term() + +

This callback is optional, so callback modules need not export it. + If a release upgrade/downgrade with Change={advanced,Extra} + specified in the appup file is made when code_change/3 + isn't implemented the process will crash with an undef exit + reason.

+

This function is called by a gen_server process when it is to update its internal state during a release upgrade/downgrade, that is, when the instruction {update,Module,Change,...}, @@ -690,6 +697,12 @@ gen_server:abcast -----> Module:handle_cast/2  Reason = normal | term() + +

This callback is optional, so callback modules need not + export it. The gen_server module provides a default + implementation of this function that logs about the unexpected + Info message, drops it and returns {noreply, State}.

+

This function is called by a gen_server process when a time-out occurs or when it receives any other message than a synchronous or asynchronous request (or a system message).

@@ -750,6 +763,11 @@ gen_server:abcast -----> Module:handle_cast/2 State = term() + +

This callback is optional, so callback modules need not + export it. The gen_server module provides a default + implementation without cleanup.

+

This function is called by a gen_server process when it is about to terminate. It is to be the opposite of Module:init/1 diff --git a/lib/stdlib/src/gen_server.erl b/lib/stdlib/src/gen_server.erl index 284810c971..8504af86f8 100644 --- a/lib/stdlib/src/gen_server.erl +++ b/lib/stdlib/src/gen_server.erl @@ -146,8 +146,8 @@ State :: term(), Status :: term(). --optional_callbacks([format_status/2]). - +-optional_callbacks( + [handle_info/2, terminate/2, code_change/3, format_status/2]). %%% ----------------------------------------------------------------- %%% Starts a generic server. @@ -602,6 +602,17 @@ try_dispatch(Mod, Func, Msg, State) -> catch throw:R -> {ok, R}; + error:undef = R when Func == handle_info -> + case erlang:function_exported(Mod, handle_info, 2) of + false -> + error_logger:warning_msg("** Undefined handle_info in ~p~n" + "** Unhandled message: ~p~n", + [Mod, Msg]), + {ok, {noreply, State}}; + true -> + Stacktrace = erlang:get_stacktrace(), + {'EXIT', {R, Stacktrace}, {R, Stacktrace}} + end; error:R -> Stacktrace = erlang:get_stacktrace(), {'EXIT', {R, Stacktrace}, {R, Stacktrace}}; @@ -625,17 +636,22 @@ try_handle_call(Mod, Msg, From, State) -> end. try_terminate(Mod, Reason, State) -> - try - {ok, Mod:terminate(Reason, State)} - catch - throw:R -> - {ok, R}; - error:R -> - Stacktrace = erlang:get_stacktrace(), - {'EXIT', {R, Stacktrace}, {R, Stacktrace}}; - exit:R -> - Stacktrace = erlang:get_stacktrace(), - {'EXIT', R, {R, Stacktrace}} + case erlang:function_exported(Mod, terminate, 2) of + true -> + try + {ok, Mod:terminate(Reason, State)} + catch + throw:R -> + {ok, R}; + error:R -> + Stacktrace = erlang:get_stacktrace(), + {'EXIT', {R, Stacktrace}, {R, Stacktrace}}; + exit:R -> + Stacktrace = erlang:get_stacktrace(), + {'EXIT', R, {R, Stacktrace}} + end; + false -> + {ok, ok} end. diff --git a/lib/stdlib/test/erl_internal_SUITE.erl b/lib/stdlib/test/erl_internal_SUITE.erl index bfa48de6b7..23d011e271 100644 --- a/lib/stdlib/test/erl_internal_SUITE.erl +++ b/lib/stdlib/test/erl_internal_SUITE.erl @@ -97,7 +97,7 @@ callbacks(supervisor) -> optional_callbacks(application) -> []; optional_callbacks(gen_server) -> - [{format_status,2}]; + [{handle_info, 2}, {terminate, 2}, {code_change, 3}, {format_status, 2}]; optional_callbacks(gen_fsm) -> [{format_status,2}]; optional_callbacks(gen_event) -> diff --git a/lib/stdlib/test/erl_lint_SUITE.erl b/lib/stdlib/test/erl_lint_SUITE.erl index c7dcd9ae16..ef4d363d29 100644 --- a/lib/stdlib/test/erl_lint_SUITE.erl +++ b/lib/stdlib/test/erl_lint_SUITE.erl @@ -3057,10 +3057,7 @@ behaviour_multiple(Config) when is_list(Config) -> handle_info(_, _) -> ok. ">>, [], - {warnings,[{1,erl_lint, - {undefined_behaviour_func,{code_change,3},gen_server}}, - {1,erl_lint,{undefined_behaviour_func,{init,1},gen_server}}, - {1,erl_lint,{undefined_behaviour_func,{terminate,2},gen_server}}, + {warnings,[{1,erl_lint,{undefined_behaviour_func,{init,1},gen_server}}, {2,erl_lint,{undefined_behaviour_func,{init,1},supervisor}}, {2, erl_lint, @@ -3074,10 +3071,7 @@ behaviour_multiple(Config) when is_list(Config) -> handle_info(_, _) -> ok. ">>, [], - {warnings,[{1,erl_lint, - {undefined_behaviour_func,{code_change,3},gen_server}}, - {1,erl_lint,{undefined_behaviour_func,{init,1},gen_server}}, - {1,erl_lint,{undefined_behaviour_func,{terminate,2},gen_server}}, + {warnings,[{1,erl_lint,{undefined_behaviour_func,{init,1},gen_server}}, {2,erl_lint,{undefined_behaviour_func,{init,1},supervisor}}, {2, erl_lint, diff --git a/lib/stdlib/test/gen_server_SUITE.erl b/lib/stdlib/test/gen_server_SUITE.erl index 6888cb8c58..3fb9b3627b 100644 --- a/lib/stdlib/test/gen_server_SUITE.erl +++ b/lib/stdlib/test/gen_server_SUITE.erl @@ -34,7 +34,10 @@ spec_init_global_registered_parent/1, otp_5854/1, hibernate/1, otp_7669/1, call_format_status/1, error_format_status/1, terminate_crash_format/1, - get_state/1, replace_state/1, call_with_huge_message_queue/1 + get_state/1, replace_state/1, call_with_huge_message_queue/1, + undef_handle_call/1, undef_handle_cast/1, undef_handle_info/1, + undef_init/1, undef_code_change/1, undef_terminate1/1, + undef_terminate2/1, undef_in_terminate/1, undef_in_handle_info/1 ]). -export([stop1/1, stop2/1, stop3/1, stop4/1, stop5/1, stop6/1, stop7/1, @@ -50,7 +53,7 @@ %% The gen_server behaviour -export([init/1, handle_call/3, handle_cast/2, - handle_info/2, terminate/2, format_status/2]). + handle_info/2, code_change/3, terminate/2, format_status/2]). suite() -> [{ct_hooks,[ts_install_cth]}, @@ -66,11 +69,16 @@ all() -> otp_7669, call_format_status, error_format_status, terminate_crash_format, get_state, replace_state, - call_with_huge_message_queue]. + call_with_huge_message_queue, {group, undef_callbacks}, + undef_in_terminate, undef_in_handle_info]. groups() -> [{stop, [], - [stop1, stop2, stop3, stop4, stop5, stop6, stop7, stop8, stop9, stop10]}]. + [stop1, stop2, stop3, stop4, stop5, stop6, stop7, stop8, stop9, stop10]}, + {undef_callbacks, [], + [undef_handle_call, undef_handle_cast, undef_handle_info, + undef_init, undef_code_change, undef_terminate1, undef_terminate2]}]. + init_per_suite(Config) -> Config. @@ -78,6 +86,11 @@ init_per_suite(Config) -> end_per_suite(_Config) -> ok. +init_per_group(undef_callbacks, Config) -> + DataDir = ?config(data_dir, Config), + Server = filename:join(DataDir, "oc_server.erl"), + {ok, oc_server} = compile:file(Server), + Config; init_per_group(_GroupName, Config) -> Config. @@ -93,6 +106,7 @@ init_per_testcase(Case, Config) when Case == call_remote1; Case == call_remote_n3 -> {ok,N} = start_node(hubba), [{node,N} | Config]; + init_per_testcase(_Case, Config) -> Config. @@ -1260,6 +1274,141 @@ echo_loop() -> echo_loop() end. +%% Test the default implementation of terminate if the callback module +%% does not export it +undef_terminate1(Config) when is_list(Config) -> + {ok, Server} = gen_server:start(oc_server, [], []), + MRef = monitor(process, Server), + ok = gen_server:stop(Server), + ok = verify_down_reason(MRef, Server, normal). + +%% Test the default implementation of terminate if the callback module +%% does not export it +undef_terminate2(Config) when is_list(Config) -> + {ok, Server} = gen_server:start(oc_server, [], []), + MRef = monitor(process, Server), + ok = gen_server:stop(Server, {error, test}, infinity), + ok = verify_down_reason(MRef, Server, {error, test}). + +%% Start should return an undef error if init isn't implemented +undef_init(_Config) -> + {error, {undef, [{oc_init_server, init, [_], _}|_]}} = + gen_server:start(oc_init_server, [], []), + process_flag(trap_exit, true), + {error, {undef, [{oc_init_server, init, [_], _}|_]}} = + (catch gen_server:start_link(oc_init_server, [], [])), + receive + {'EXIT', Server, + {undef, [{oc_init_server, init, [_], _}|_]}} when is_pid(Server) -> + ok + after 1000 -> + ct:fail(expected_exit_msg) + end. + +%% The upgrade should fail if code_change is expected in the callback module +%% but not exported, but the server should continue with the old code +undef_code_change(Config) when is_list(Config) -> + {ok, Server} = gen_server:start(oc_server, [], []), + {error, {'EXIT', {undef, [{oc_server, code_change, [_, _, _], _}|_]}}} + = fake_upgrade(Server, ?MODULE), + true = is_process_alive(Server). + +%% The server should crash if the handle_call callback is +%% not exported in the callback module +undef_handle_call(_Config) -> + {ok, Server} = gen_server:start(oc_server, [], []), + try + gen_server:call(Server, call_msg), + ct:fail(should_crash) + catch exit:{{undef, [{oc_server, handle_call, _, _}|_]}, + {gen_server, call, _}} -> + ok + end. + +%% The server should crash if the handle_cast callback is +%% not exported in the callback module +undef_handle_cast(_Config) -> + {ok, Server} = gen_server:start(oc_server, [], []), + MRef = monitor(process, Server), + gen_server:cast(Server, cast_msg), + verify_undef_down(MRef, Server, oc_server, handle_cast), + ok. + +%% The server should log but not crash if the handle_info callback is +%% calling an undefined function +undef_handle_info(Config) when is_list(Config) -> + error_logger_forwarder:register(), + {ok, Server} = gen_server:start(oc_server, [], []), + Server ! hej, + wait_until_processed(Server, hej, 10), + true = is_process_alive(Server), + receive + {warning_msg, _GroupLeader, + {Server, "** Undefined handle_info in " ++ _, [oc_server, hej]}} -> + ok; + Other -> + io:format("Unexpected: ~p", [Other]), + ct:fail(failed) + end. + +%% Test that the default implementation of terminate isn't catching the +%% wrong undef error +undef_in_terminate(Config) when is_list(Config) -> + State = {undef_in_terminate, {oc_server, terminate}}, + {ok, Server} = gen_server:start(?MODULE, {state, State}, []), + try + gen_server:stop(Server), + ct:fail(failed) + catch + exit:{undef, [{oc_server, terminate, [], _}|_]} -> + ok + end. + +%% Test that the default implementation of handle_info isn't catching the +%% wrong undef error +undef_in_handle_info(Config) when is_list(Config) -> + {ok, Server} = gen_server:start(?MODULE, [], []), + MRef = monitor(process, Server), + Server ! {call_undef_fun, ?MODULE, handle_info}, + verify_undef_down(MRef, Server, ?MODULE, handle_info), + ok. + +verify_down_reason(MRef, Server, Reason) -> + receive + {'DOWN', MRef, process, Server, Reason} -> + ok + after 5000 -> + ct:fail(failed) + end. + +verify_undef_down(MRef, Pid, Mod, Fun) -> + ok = receive + {'DOWN', MRef, process, Pid, + {undef, [{Mod, Fun, _, _}|_]}} -> + ok + after 5000 -> + ct:fail(should_crash) + end. + +fake_upgrade(Pid, Mod) -> + sys:suspend(Pid), + sys:replace_state(Pid, fun(State) -> {new, State} end), + Ret = sys:change_code(Pid, Mod, old_vsn, []), + ok = sys:resume(Pid), + Ret. + +wait_until_processed(_Pid, _Message, 0) -> + ct:fail(not_processed); +wait_until_processed(Pid, Message, N) -> + {messages, Messages} = erlang:process_info(Pid, messages), + case lists:member(Message, Messages) of + true -> + timer:sleep(100), + wait_until_processed(Pid, Message, N-1); + false -> + ok + end. + %%-------------------------------------------------------------- %% Help functions to spec_init_* start_link(Init, Options) -> @@ -1383,6 +1532,9 @@ handle_call(stop_shutdown, _From, State) -> {stop,shutdown,State}; handle_call(shutdown_reason, _From, _State) -> exit({shutdown,reason}); +handle_call({call_undef_fun, Mod, Fun}, _From, State) -> + Mod:Fun(), + {reply, ok, State}; handle_call(stop_shutdown_reason, _From, State) -> {stop,{shutdown,stop_reason},State}. @@ -1396,6 +1548,9 @@ handle_cast(hibernate_now, _State) -> handle_cast(hibernate_later, _State) -> timer:send_after(1000,self(),hibernate_now), {noreply, []}; +handle_cast({call_undef_fun, Mod, Fun}, State) -> + Mod:Fun(), + {noreply, State}; handle_cast({From, stop}, State) -> io:format("BAZ"), {stop, {From,stopped}, State}. @@ -1420,6 +1575,9 @@ handle_info(timeout, {delayed_cast, From}) -> handle_info(timeout, {delayed_info, From}) -> From ! {self(), delayed_info}, {noreply, []}; +handle_info({call_undef_fun, Mod, Fun}, State) -> + Mod:Fun(), + {noreply, State}; handle_info({From, handle_info}, _State) -> From ! {self(), handled_info}, {noreply, []}; @@ -1433,6 +1591,12 @@ handle_info({From, stop}, State) -> handle_info(_Info, State) -> {noreply, State}. +code_change(_OldVsn, + {new, {undef_in_code_change, {Mod, Fun}}} = State, + _Extra) -> + Mod:Fun(), + {ok, State}. + terminate({From, stopped}, _State) -> io:format("FOOBAR"), From ! {self(), stopped}, @@ -1442,6 +1606,9 @@ terminate({From, stopped_info}, _State) -> ok; terminate(_, crash_terminate) -> exit({crash, terminate}); +terminate(_, {undef_in_terminate, {Mod, Fun}}) -> + Mod:Fun(), + ok; terminate(_Reason, _State) -> ok. diff --git a/lib/stdlib/test/gen_server_SUITE_data/oc_server.erl b/lib/stdlib/test/gen_server_SUITE_data/oc_server.erl new file mode 100644 index 0000000000..4ba37987f3 --- /dev/null +++ b/lib/stdlib/test/gen_server_SUITE_data/oc_server.erl @@ -0,0 +1,37 @@ +%% +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2017. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% +%% +-module(oc_server). + +-behaviour(gen_server). + +%% API +-export([start/0]). + +%% gen_server callbacks +-export([init/1]). + +-record(state, {}). + +start() -> + gen_server:start({local, ?MODULE}, ?MODULE, [], []). + +init([]) -> + {ok, #state{}}. + -- cgit v1.2.3 From db632b91f41c4a01be68d821d6942ff32b86e40b Mon Sep 17 00:00:00 2001 From: Zandra Norman Date: Mon, 23 Jan 2017 16:07:52 +0100 Subject: stdlib: Make gen_event callbacks optional --- lib/stdlib/doc/src/gen_event.xml | 18 +++ lib/stdlib/src/gen_event.erl | 19 +++- lib/stdlib/test/dummy_h.erl | 6 +- lib/stdlib/test/erl_internal_SUITE.erl | 2 +- lib/stdlib/test/gen_event_SUITE.erl | 132 +++++++++++++++++++++- lib/stdlib/test/gen_event_SUITE_data/oc_event.erl | 40 +++++++ 6 files changed, 207 insertions(+), 10 deletions(-) create mode 100644 lib/stdlib/test/gen_event_SUITE_data/oc_event.erl (limited to 'lib/stdlib') diff --git a/lib/stdlib/doc/src/gen_event.xml b/lib/stdlib/doc/src/gen_event.xml index 42e952fd46..56cb7974a2 100644 --- a/lib/stdlib/doc/src/gen_event.xml +++ b/lib/stdlib/doc/src/gen_event.xml @@ -579,6 +579,13 @@ gen_event:stop -----> Module:terminate/2 Extra = term() + +

This callback is optional, so callback modules need not export it. + If a release upgrade/downgrade with Change={advanced,Extra} + specified in the .appup file is made when code_change/3 + isn't implemented the event handler will crash with an undef error + reason.

+

This function is called for an installed event handler that is to update its internal state during a release upgrade/downgrade, that is, when the instruction @@ -759,6 +766,12 @@ gen_event:stop -----> Module:terminate/2   Id = term() + +

This callback is optional, so callback modules need not + export it. The gen_event module provides a default + implementation of this function that logs about the unexpected + Info message, drops it and returns {noreply, State}.

+

This function is called for each installed event handler when an event manager receives any other message than an event or a synchronous request (or a system message).

@@ -815,6 +828,11 @@ gen_event:stop -----> Module:terminate/2  Args = Reason = Term = term() + +

This callback is optional, so callback modules need not + export it. The gen_event module provides a default + implementation without cleanup.

+

Whenever an event handler is deleted from an event manager, this function is called. It is to be the opposite of Module:init/1 diff --git a/lib/stdlib/src/gen_event.erl b/lib/stdlib/src/gen_event.erl index 0aebf1bdc5..0c50b2aa08 100644 --- a/lib/stdlib/src/gen_event.erl +++ b/lib/stdlib/src/gen_event.erl @@ -109,7 +109,8 @@ State :: term(), Status :: term(). --optional_callbacks([format_status/2]). +-optional_callbacks( + [handle_info/2, terminate/2, code_change/3, format_status/2]). %%--------------------------------------------------------------------------- @@ -577,6 +578,10 @@ server_update(Handler1, Func, Event, SName) -> do_terminate(Mod1, Handler1, remove_handler, State, remove, SName, normal), no; + {'EXIT', {undef, [{Mod1, handle_info, [_,_], _}|_]}} -> + error_logger:warning_msg("** Undefined handle_info in ~p~n" + "** Unhandled message: ~p~n", [Mod1, Event]), + {ok, Handler1}; Other -> do_terminate(Mod1, Handler1, {error, Other}, State, Event, SName, crash), @@ -698,9 +703,15 @@ server_call_update(Handler1, Query, SName) -> end. do_terminate(Mod, Handler, Args, State, LastIn, SName, Reason) -> - Res = (catch Mod:terminate(Args, State)), - report_terminate(Handler, Reason, Args, State, LastIn, SName, Res), - Res. + case erlang:function_exported(Mod, terminate, 2) of + true -> + Res = (catch Mod:terminate(Args, State)), + report_terminate(Handler, Reason, Args, State, LastIn, SName, Res), + Res; + false -> + report_terminate(Handler, Reason, Args, State, LastIn, SName, ok), + ok + end. report_terminate(Handler, crash, {error, Why}, State, LastIn, SName, _) -> report_terminate(Handler, Why, State, LastIn, SName); diff --git a/lib/stdlib/test/dummy_h.erl b/lib/stdlib/test/dummy_h.erl index bc89cb4fde..70c0eafbdf 100644 --- a/lib/stdlib/test/dummy_h.erl +++ b/lib/stdlib/test/dummy_h.erl @@ -26,6 +26,8 @@ init(make_error) -> {error, my_error}; +init({state, State}) -> + {ok, State}; init([Parent]) -> {ok, Parent}; %% We will send special responses for every handled event. init([Parent,hibernate]) -> @@ -83,7 +85,9 @@ terminate(swap, State) -> {ok, State}; terminate({error, {return, faulty}}, Parent) -> Parent ! {dummy_h, returned_error}; +terminate(_Reason, {undef_in_terminate, {Mod, Fun}}) -> + Mod:Fun(), + ok; terminate(_Reason, _State) -> ok. - diff --git a/lib/stdlib/test/erl_internal_SUITE.erl b/lib/stdlib/test/erl_internal_SUITE.erl index 23d011e271..c4c4a0408b 100644 --- a/lib/stdlib/test/erl_internal_SUITE.erl +++ b/lib/stdlib/test/erl_internal_SUITE.erl @@ -101,7 +101,7 @@ optional_callbacks(gen_server) -> optional_callbacks(gen_fsm) -> [{format_status,2}]; optional_callbacks(gen_event) -> - [{format_status,2}]; + [{handle_info, 2}, {terminate, 2}, {code_change, 3}, {format_status, 2}]; optional_callbacks(supervisor_bridge) -> []; optional_callbacks(supervisor) -> diff --git a/lib/stdlib/test/gen_event_SUITE.erl b/lib/stdlib/test/gen_event_SUITE.erl index 9a7400c84e..3f949ca109 100644 --- a/lib/stdlib/test/gen_event_SUITE.erl +++ b/lib/stdlib/test/gen_event_SUITE.erl @@ -22,13 +22,17 @@ -include_lib("common_test/include/ct.hrl"). -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, - init_per_group/2,end_per_group/2]). + init_per_group/2,end_per_group/2, init_per_testcase/2, + end_per_testcase/2]). -export([start/1, add_handler/1, add_sup_handler/1, delete_handler/1, swap_handler/1, swap_sup_handler/1, notify/1, sync_notify/1, call/1, info/1, hibernate/1, call_format_status/1, call_format_status_anon/1, error_format_status/1, get_state/1, replace_state/1, - start_opt/1]). + start_opt/1, + undef_init/1, undef_handle_call/1, undef_handle_event/1, + undef_handle_info/1, undef_code_change/1, undef_terminate/1, + undef_in_terminate/1]). suite() -> [{ct_hooks,[ts_install_cth]}]. @@ -36,13 +40,16 @@ all() -> [start, {group, test_all}, hibernate, call_format_status, call_format_status_anon, error_format_status, get_state, replace_state, - start_opt]. + start_opt, {group, undef_callbacks}, undef_in_terminate]. groups() -> [{test_all, [], [add_handler, add_sup_handler, delete_handler, swap_handler, swap_sup_handler, notify, sync_notify, - call, info]}]. + call, info]}, + {undef_callbacks, [], + [undef_init, undef_handle_call, undef_handle_event, undef_handle_info, + undef_code_change, undef_terminate]}]. init_per_suite(Config) -> Config. @@ -50,12 +57,40 @@ init_per_suite(Config) -> end_per_suite(_Config) -> ok. +init_per_group(undef_callbacks, Config) -> + DataDir = ?config(data_dir, Config), + Event1 = filename:join(DataDir, "oc_event.erl"), + {ok, oc_event} = compile:file(Event1), + Config; init_per_group(_GroupName, Config) -> Config. end_per_group(_GroupName, Config) -> Config. +init_per_testcase(Case, Config) when Case == undef_handle_call; + Case == undef_handle_info; + Case == undef_handle_event; + Case == undef_code_change; + Case == undef_terminate -> + {ok, Pid} = oc_event:start(), + [{event_pid, Pid}|Config]; +init_per_testcase(undef_init, Config) -> + {ok, Pid} = gen_event:start({local, oc_init_event}), + [{event_pid, Pid}|Config]; +init_per_testcase(_Case, Config) -> + Config. + +end_per_testcase(Case, Config) when Case == undef_init; + Case == undef_handle_call; + Case == undef_handle_info; + Case == undef_handle_event; + Case == undef_code_change; + Case == undef_terminate -> + Pid = ?config(event_pid, Config), + gen_event:stop(Pid); +end_per_testcase(_Case, _Config) -> + ok. %% -------------------------------------- %% Start an event manager. @@ -1055,3 +1090,92 @@ replace_state(Config) when is_list(Config) -> ok = sys:resume(Pid), [{dummy1_h,false,NState3}] = sys:get_state(Pid), ok. + +%% No default provided for init, so it should fail +undef_init(Config) -> + Pid = ?config(event_pid, Config), + {'EXIT', {undef, [{oc_init_event, init, [_], _}|_]}} + = gen_event:add_handler(Pid, oc_init_event, []), + ok. + +%% No default provided for init, so it should fail +undef_handle_call(Config) when is_list(Config) -> + Pid = ?config(event_pid, Config), + {error, {'EXIT', {undef, [{oc_event, handle_call, _, _}|_]}}} + = gen_event:call(Pid, oc_event, call_msg), + [] = gen_event:which_handlers(Pid), + ok. + +%% No default provided for init, so it should fail +undef_handle_event(Config) -> + Pid = ?config(event_pid, Config), + ok = gen_event:sync_notify(Pid, event_msg), + [] = gen_event:which_handlers(Pid), + + gen_event:add_handler(oc_event, oc_event, []), + [oc_event] = gen_event:which_handlers(Pid), + + ok = gen_event:notify(Pid, event_msg), + [] = gen_event:which_handlers(Pid), + ok. + +%% Defaulting to doing nothing with a log warning. +undef_handle_info(Config) when is_list(Config) -> + error_logger_forwarder:register(), + Pid = ?config(event_pid, Config), + Pid ! hej, + wait_until_processed(Pid, hej, 10), + [oc_event] = gen_event:which_handlers(Pid), + receive + {warning_msg, _GroupLeader, + {Pid, "** Undefined handle_info in " ++ _, [oc_event, hej]}} -> + ok; + Other -> + io:format("Unexpected: ~p", [Other]), + ct:fail(failed) + end. + +wait_until_processed(_Pid, _Message, 0) -> + ct:fail(not_processed); +wait_until_processed(Pid, Message, N) -> + {messages, Messages} = erlang:process_info(Pid, messages), + case lists:member(Message, Messages) of + true -> + timer:sleep(100), + wait_until_processed(Pid, Message, N-1); + false -> + ok + end. + +%% No default provided for init, so it should fail +undef_code_change(Config) when is_list(Config) -> + Pid = ?config(event_pid, Config), + {error, {'EXIT', {undef, [{oc_event, code_change, [_, _, _], _}|_]}}} = + fake_upgrade(Pid, oc_event), + [oc_event] = gen_event:which_handlers(Pid), + ok. + +%% Defaulting to doing nothing. Test that it works when not defined. +undef_terminate(Config) when is_list(Config) -> + Pid = ?config(event_pid, Config), + ok = gen_event:delete_handler(Pid, oc_event, []), + [] = gen_event:which_handlers(Pid), + ok. + +%% Test that the default implementation doesn't catch the wrong undef error +undef_in_terminate(_Config) -> + {ok, Pid} = gen_event:start({local, dummy}), + State = {undef_in_terminate, {dummy_h, terminate}}, + ok = gen_event:add_handler(Pid, dummy_h, {state, State}), + [dummy_h] = gen_event:which_handlers(Pid), + {'EXIT', {undef, [{dummy_h, terminate, [], []}|_]}} + = gen_event:delete_handler(Pid, dummy_h, []), + [] = gen_event:which_handlers(Pid), + ok. + +fake_upgrade(Pid, Mod) -> + sys:suspend(Pid), + sys:replace_state(Pid, fun(S) -> {new, S} end), + Ret = sys:change_code(Pid, Mod, old_vsn, []), + ok = sys:resume(Pid), + Ret. diff --git a/lib/stdlib/test/gen_event_SUITE_data/oc_event.erl b/lib/stdlib/test/gen_event_SUITE_data/oc_event.erl new file mode 100644 index 0000000000..eb664ef550 --- /dev/null +++ b/lib/stdlib/test/gen_event_SUITE_data/oc_event.erl @@ -0,0 +1,40 @@ +%% +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2017. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% +%% +-module(oc_event). + +-behaviour(gen_event). + +%% API +-export([start/0]). + +%% gen_event callbacks +-export([init/1]). + +-define(SERVER, ?MODULE). + +-record(state, {}). + +start() -> + {ok, Pid} = gen_event:start({local, ?SERVER}), + gen_event:add_handler(?SERVER, ?MODULE, []), + {ok, Pid}. + +init([]) -> + {ok, #state{}}. -- cgit v1.2.3 From 215f61f223a07493d1147a04375d11f3c7819e42 Mon Sep 17 00:00:00 2001 From: Zandra Norman Date: Tue, 24 Jan 2017 13:40:21 +0100 Subject: stdlib: Make gen_fsm callbacks optional --- lib/stdlib/doc/src/gen_fsm.xml | 19 ++++ lib/stdlib/src/gen_fsm.erl | 47 +++++--- lib/stdlib/test/erl_internal_SUITE.erl | 2 +- lib/stdlib/test/gen_fsm_SUITE.erl | 151 +++++++++++++++++++++++++- lib/stdlib/test/gen_fsm_SUITE_data/oc_fsm.erl | 48 ++++++++ 5 files changed, 244 insertions(+), 23 deletions(-) create mode 100644 lib/stdlib/test/gen_fsm_SUITE_data/oc_fsm.erl (limited to 'lib/stdlib') diff --git a/lib/stdlib/doc/src/gen_fsm.xml b/lib/stdlib/doc/src/gen_fsm.xml index 719ab2b558..691a039e34 100644 --- a/lib/stdlib/doc/src/gen_fsm.xml +++ b/lib/stdlib/doc/src/gen_fsm.xml @@ -562,6 +562,13 @@ gen_fsm:sync_send_all_state_event -----> Module:handle_sync_event/4 Extra = term() + +

This callback is optional, so callback modules need not export it. + If a release upgrade/downgrade with Change={advanced,Extra} + specified in the appup file is made when code_change/4 + isn't implemented the process will crash with an undef exit + reason.

+

This function is called by a gen_fsm process when it is to update its internal state data during a release upgrade/downgrade, that is, when instruction {update,Module,Change,...}, @@ -686,6 +693,13 @@ gen_fsm:sync_send_all_state_event -----> Module:handle_sync_event/4  Reason = normal | term() + +

This callback is optional, so callback modules need not + export it. The gen_fsm module provides a default + implementation of this function that logs about the unexpected + Info message, drops it and returns + {next_state, StateName, StateData}.

+

This function is called by a gen_fsm process when it receives any other message than a synchronous or asynchronous event (or a system message).

@@ -899,6 +913,11 @@ gen_fsm:sync_send_all_state_event -----> Module:handle_sync_event/4 StateData = term() + +

This callback is optional, so callback modules need not + export it. The gen_fsm module provides a default + implementation without cleanup.

+

This function is called by a gen_fsm process when it is about to terminate. It is to be the opposite of Module:init/1 diff --git a/lib/stdlib/src/gen_fsm.erl b/lib/stdlib/src/gen_fsm.erl index e925a75fe8..39a8fd42fe 100644 --- a/lib/stdlib/src/gen_fsm.erl +++ b/lib/stdlib/src/gen_fsm.erl @@ -169,7 +169,8 @@ State :: term(), Status :: term(). --optional_callbacks([format_status/2]). +-optional_callbacks( + [handle_info/3, terminate/3, code_change/4, format_status/2]). %%% --------------------------------------------------- %%% Starts a generic state machine. @@ -466,6 +467,10 @@ handle_msg(Msg, Parent, Name, StateName, StateData, Mod, _Time) -> %No debug her StateName, NStateData, [])), reply(From, Reply), exit(R); + {'EXIT', {undef, [{Mod, handle_info, [_,_,_], _}|_]}} -> + error_logger:warning_msg("** Undefined handle_info in ~p~n" + "** Unhandled message: ~p~n", [Mod, Msg]), + loop(Parent, Name, StateName, StateData, Mod, infinity, []); {'EXIT', What} -> terminate(What, Name, Msg, Mod, StateName, StateData, []); Reply -> @@ -540,24 +545,30 @@ reply(Name, {To, Tag}, Reply, Debug, StateName) -> -spec terminate(term(), _, _, atom(), _, _, _) -> no_return(). terminate(Reason, Name, Msg, Mod, StateName, StateData, Debug) -> - case catch Mod:terminate(Reason, StateName, StateData) of - {'EXIT', R} -> - FmtStateData = format_status(terminate, Mod, get(), StateData), - error_info(R, Name, Msg, StateName, FmtStateData, Debug), - exit(R); - _ -> - case Reason of - normal -> - exit(normal); - shutdown -> - exit(shutdown); - {shutdown,_}=Shutdown -> - exit(Shutdown); + case erlang:function_exported(Mod, terminate, 3) of + true -> + case catch Mod:terminate(Reason, StateName, StateData) of + {'EXIT', R} -> + FmtStateData = format_status(terminate, Mod, get(), StateData), + error_info(R, Name, Msg, StateName, FmtStateData, Debug), + exit(R); _ -> - FmtStateData = format_status(terminate, Mod, get(), StateData), - error_info(Reason,Name,Msg,StateName,FmtStateData,Debug), - exit(Reason) - end + ok + end; + false -> + ok + end, + case Reason of + normal -> + exit(normal); + shutdown -> + exit(shutdown); + {shutdown,_}=Shutdown -> + exit(Shutdown); + _ -> + FmtStateData1 = format_status(terminate, Mod, get(), StateData), + error_info(Reason,Name,Msg,StateName,FmtStateData1,Debug), + exit(Reason) end. error_info(Reason, Name, Msg, StateName, StateData, Debug) -> diff --git a/lib/stdlib/test/erl_internal_SUITE.erl b/lib/stdlib/test/erl_internal_SUITE.erl index c4c4a0408b..099f21f905 100644 --- a/lib/stdlib/test/erl_internal_SUITE.erl +++ b/lib/stdlib/test/erl_internal_SUITE.erl @@ -99,7 +99,7 @@ optional_callbacks(application) -> optional_callbacks(gen_server) -> [{handle_info, 2}, {terminate, 2}, {code_change, 3}, {format_status, 2}]; optional_callbacks(gen_fsm) -> - [{format_status,2}]; + [{handle_info, 3}, {terminate, 3}, {code_change, 4}, {format_status, 2}]; optional_callbacks(gen_event) -> [{handle_info, 2}, {terminate, 2}, {code_change, 3}, {format_status, 2}]; optional_callbacks(supervisor_bridge) -> diff --git a/lib/stdlib/test/gen_fsm_SUITE.erl b/lib/stdlib/test/gen_fsm_SUITE.erl index d6bb002b5f..361680a9b2 100644 --- a/lib/stdlib/test/gen_fsm_SUITE.erl +++ b/lib/stdlib/test/gen_fsm_SUITE.erl @@ -39,6 +39,11 @@ call_format_status/1, error_format_status/1, terminate_crash_format/1, get_state/1, replace_state/1]). +-export([undef_handle_event/1, undef_handle_sync_event/1, undef_handle_info/1, + undef_init/1, undef_code_change/1, undef_terminate1/1, undef_terminate2/1]). + +-export([undef_in_handle_info/1, undef_in_terminate/1]). + -export([hibernate/1,hiber_idle/3,hiber_wakeup/3,hiber_idle/2,hiber_wakeup/2]). -export([enter_loop/1]). @@ -48,7 +53,7 @@ %% The gen_fsm behaviour -export([init/1, handle_event/3, handle_sync_event/4, terminate/3, - handle_info/3, format_status/2]). + handle_info/3, format_status/2, code_change/4]). -export([idle/2, idle/3, timeout/2, wfor_conf/2, wfor_conf/3, @@ -63,7 +68,8 @@ suite() -> [{ct_hooks,[ts_install_cth]}]. all() -> [{group, start}, {group, abnormal}, shutdown, - {group, sys}, hibernate, enter_loop]. + {group, sys}, hibernate, enter_loop, {group, undef_callbacks}, + undef_in_handle_info, undef_in_terminate]. groups() -> [{start, [], @@ -74,7 +80,10 @@ groups() -> {abnormal, [], [abnormal1, abnormal2]}, {sys, [], [sys1, call_format_status, error_format_status, terminate_crash_format, - get_state, replace_state]}]. + get_state, replace_state]}, + {undef_callbacks, [], + [undef_handle_event, undef_handle_sync_event, undef_handle_info, + undef_init, undef_code_change, undef_terminate1, undef_terminate2]}]. init_per_suite(Config) -> Config. @@ -82,6 +91,11 @@ init_per_suite(Config) -> end_per_suite(_Config) -> ok. +init_per_group(undef_callbacks, Config) -> + DataDir = ?config(data_dir, Config), + Server = filename:join(DataDir, "oc_fsm.erl"), + {ok, oc_fsm} = compile:file(Server), + Config; init_per_group(_GroupName, Config) -> Config. @@ -868,6 +882,99 @@ enter_loop(Reg1, Reg2) -> gen_fsm:enter_loop(?MODULE, [], state0, []) end. +%% Start should return an undef error if init isn't implemented +undef_init(Config) when is_list(Config) -> + {error, {undef, [{oc_init_fsm, init, [[]], []}|_]}} + = gen_fsm:start(oc_init_fsm, [], []), + ok. + +%% Test that the server crashes correctly if the handle_event callback is +%% not exported in the callback module +undef_handle_event(Config) when is_list(Config) -> + {ok, FSM} = gen_fsm:start(oc_fsm, [], []), + MRef = monitor(process, FSM), + gen_fsm:send_all_state_event(FSM, state_name), + ok = verify_undef_down(MRef, FSM, oc_fsm, handle_event). + +%% Test that the server crashes correctly if the handle_sync_event callback is +%% not exported in the callback module +undef_handle_sync_event(Config) when is_list(Config) -> + {ok, FSM} = gen_fsm:start(oc_fsm, [], []), + try + gen_fsm:sync_send_all_state_event(FSM, state_name), + ct:fail(should_crash) + catch exit:{{undef, [{oc_fsm, handle_sync_event, _, _}|_]},_} -> + ok + end. + +%% The fsm should log but not crash if the handle_info callback is +%% calling an undefined function +undef_handle_info(Config) when is_list(Config) -> + error_logger_forwarder:register(), + {ok, FSM} = gen_fsm:start(oc_fsm, [], []), + MRef = monitor(process, FSM), + FSM ! hej, + receive + {'DOWN', MRef, process, FSM, _} -> + ct:fail(should_not_crash) + after 500 -> + ok + end, + receive + {warning_msg, _GroupLeader, + {FSM, "** Undefined handle_info in " ++ _, [oc_fsm, hej]}} -> + ok; + Other -> + io:format("Unexpected: ~p", [Other]), + ct:fail(failed) + end. + +%% The upgrade should fail if code_change is expected in the callback module +%% but not exported, but the fsm should continue with the old code +undef_code_change(Config) when is_list(Config) -> + {ok, FSM} = gen_fsm:start(oc_fsm, [], []), + {error, {'EXIT', {undef, [{oc_fsm, code_change, [_, _, _, _], _}|_]}}} + = fake_upgrade(FSM, oc_fsm), + ok. + +%% Test the default implementation of terminate with normal reason if the +%% callback module does not export it +undef_terminate1(Config) when is_list(Config) -> + {ok, FSM} = gen_fsm:start(oc_fsm, [], []), + MRef = monitor(process, FSM), + ok = gen_fsm:stop(FSM), + ok = verify_down_reason(MRef, FSM, normal). + +%% Test the default implementation of terminate with error reason if the +%% callback module does not export it +undef_terminate2(Config) when is_list(Config) -> + {ok, FSM} = gen_fsm:start(oc_fsm, [], []), + MRef = monitor(process, FSM), + ok = gen_fsm:stop(FSM, {error, test}, infinity), + ok = verify_down_reason(MRef, FSM, {error, test}). + +%% Test that the server crashes correctly if the handle_info callback is +%% calling an undefined function +undef_in_handle_info(Config) when is_list(Config) -> + {ok, FSM} = gen_fsm:start(?MODULE, [], []), + MRef = monitor(process, FSM), + FSM ! {call_undef_fun, {?MODULE, handle_info}}, + verify_undef_down(MRef, FSM, ?MODULE, handle_info), + ok. + +%% Test that the server crashes correctly if the terminate callback is +%% calling an undefined function +undef_in_terminate(Config) when is_list(Config) -> + State = {undef_in_terminate, {?MODULE, terminate}}, + {ok, FSM} = gen_fsm:start(?MODULE, {state_data, State}, []), + try + gen_fsm:stop(FSM), + ct:fail(failed) + catch + exit:{undef, [{?MODULE, terminate, _, _}|_]} -> + ok + end. + %% %% Functionality check %% @@ -962,7 +1069,31 @@ do_sync_disconnect(FSM) -> yes = gen_fsm:sync_send_event(FSM, disconnect), check_state(FSM, idle). +verify_down_reason(MRef, Pid, Reason) -> + receive + {'DOWN', MRef, process, Pid, Reason} -> + ok; + {'DOWN', MRef, process, Pid, Other}-> + ct:fail({wrong_down_reason, Other}) + after 5000 -> + ct:fail(should_shutdown) + end. +verify_undef_down(MRef, Pid, Mod, Fun) -> + ok = receive + {'DOWN', MRef, process, Pid, + {undef, [{Mod, Fun, _, _}|_]}} -> + ok + after 5000 -> + ct:fail(should_crash) + end. + +fake_upgrade(Pid, Mod) -> + sys:suspend(Pid), + sys:replace_state(Pid, fun(State) -> {new, State} end), + Ret = sys:change_code(Pid, Mod, old_vsn, []), + ok = sys:resume(Pid), + Ret. %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %% @@ -992,6 +1123,9 @@ init(_) -> terminate(_, _State, crash_terminate) -> exit({crash, terminate}); +terminate(_, _, {undef_in_terminate, {Mod, Fun}}) -> + Mod:Fun(), + ok; terminate({From, stopped}, State, _Data) -> From ! {self(), {stopped, State}}, ok; @@ -1089,7 +1223,9 @@ handle_info(hibernate_now, _SName, _State) -> {next_state, hiber_idle, [], hibernate}; handle_info(hibernate_later, _SName, _State) -> {next_state, hiber_idle, hibernate_me, 1000}; - +handle_info({call_undef_fun, {Mod, Fun}}, State, Data) -> + Mod:Fun(), + {next_state, State, Data}; handle_info(Info, _State, Data) -> {stop, {unexpected,Info}, Data}. @@ -1134,6 +1270,13 @@ format_status(terminate, [_Pdict, StateData]) -> format_status(normal, [_Pdict, _StateData]) -> [format_status_called]. +code_change(_OldVsn, State, + {idle, {undef_in_code_change, {Mod, Fun}}} = Data, _Extra) -> + Mod:Fun(), + {ok, State, Data}; +code_change(_OldVsn, State, Data, _Extra) -> + {ok, State, Data}. + get_messages() -> receive Msg -> [Msg|get_messages()] diff --git a/lib/stdlib/test/gen_fsm_SUITE_data/oc_fsm.erl b/lib/stdlib/test/gen_fsm_SUITE_data/oc_fsm.erl new file mode 100644 index 0000000000..27fb1bc3b6 --- /dev/null +++ b/lib/stdlib/test/gen_fsm_SUITE_data/oc_fsm.erl @@ -0,0 +1,48 @@ +%% +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2017. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% +%% +-module(oc_fsm). + +-behaviour(gen_fsm). + +%% API +-export([start/0]). + +%% gen_fsm callbacks +-export([init/1, + state_name/2, + state_name/3]). + +-define(SERVER, ?MODULE). + +-record(state, {}). + +start() -> + gen_fsm:start({local, ?SERVER}, ?MODULE, [], []). + +init([]) -> + {ok, state_name, #state{}}. + +state_name(_Event, State) -> + {next_state, state_name, State}. + +state_name(_Event, _From, State) -> + Reply = ok, + {reply, Reply, state_name, State}. + -- cgit v1.2.3