Merge branch 'bg/on_load' into ccase/r13b04_dev

* bg/on_load:
  Test on_load functions that don't return 'ok'
  Change the expected return value for on_load functions

OTP-8339  The expected return value for an on_load function has been
          changed. (See the section about code loading in the Reference
          manual.)

11 files changed, 149 insertions, 27 deletions

diff --git a/erts/preloaded/ebin/init.beam b/erts/preloaded/ebin/init.beam
index 7b6bafd1af..be1f71e6c5 100644
--- a/erts/preloaded/ebin/init.beam
+++ b/erts/preloaded/ebin/init.beam
diff --git a/erts/preloaded/src/init.erl b/erts/preloaded/src/init.erl
index c6f4c62f63..c0b3d286e8 100644
--- a/erts/preloaded/src/init.erl
+++ b/erts/preloaded/src/init.erl
@@ -1357,10 +1357,7 @@ run_on_load_handlers([M|Ms]) ->
     {Pid,Ref} = spawn_monitor(Fun),
     receive
 	{'DOWN',Ref,process,Pid,OnLoadRes} ->
-	    Keep = if
-		       is_boolean(OnLoadRes) -> OnLoadRes;
-		       true -> false
-		   end,
+	    Keep = OnLoadRes =:= ok,
 	    erlang:finish_after_on_load(M, Keep),
 	    case Keep of
 		false ->
diff --git a/lib/compiler/test/compilation_SUITE_data/on_load.erl b/lib/compiler/test/compilation_SUITE_data/on_load.erl
index 92bcf74624..e9b5ec7f34 100644
--- a/lib/compiler/test/compilation_SUITE_data/on_load.erl
+++ b/lib/compiler/test/compilation_SUITE_data/on_load.erl
@@ -12,7 +12,7 @@
 
 do_on_load() ->
     local_function(),
-    true.
+    ok.
 
 local_function() ->
     ok.
diff --git a/lib/kernel/src/code_server.erl b/lib/kernel/src/code_server.erl
index 018f7f41d2..d4e3f0bcf8 100644
--- a/lib/kernel/src/code_server.erl
+++ b/lib/kernel/src/code_server.erl
@@ -1479,13 +1479,12 @@ finish_on_load(Ref, OnLoadRes, #state{on_load=OnLoad0,moddb=Db}=State) ->
     end.
 
 finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db) ->
-    Keep = if
-	       is_boolean(OnLoadRes) -> OnLoadRes;
-	       true -> false
-	   end,
+    Keep = OnLoadRes =:= ok,
     erlang:finish_after_on_load(Mod, Keep),
     Res = case Keep of
-	      false -> {error,on_load_failure};
+	      false ->
+		  finish_on_load_report(Mod, OnLoadRes),
+		  {error,on_load_failure};
 	      true ->
 		  ets:insert(Db, {Mod,File}),
 		  {module,Mod}
@@ -1493,6 +1492,24 @@ finish_on_load_1(Mod, File, OnLoadRes, WaitingPids, Db) ->
     [reply(Pid, Res) || Pid <- WaitingPids],
     ok.
 
+finish_on_load_report(_Mod, Atom) when is_atom(Atom) ->
+    %% No error reports for atoms.
+    ok;
+finish_on_load_report(Mod, Term) ->
+    %% Play it very safe here. The error_logger module and
+    %% modules it depend on may not be loaded yet and there
+    %% would be a dead-lock if we called it directly
+    %% from the code_server process.
+    spawn(fun() ->
+		  F = "The on_load function for module "
+		      "~s returned ~P\n",
+
+		  %% Express the call as an apply to simplify
+		  %% the ext_mod_dep/1 test case.
+		  E = error_logger,
+		  E:warning_msg(F, [Mod,Term,10])
+	  end).
+
 %% -------------------------------------------------------
 %% Internal functions.
 %% -------------------------------------------------------
diff --git a/lib/kernel/test/code_SUITE.erl b/lib/kernel/test/code_SUITE.erl
index 9fda66711d..38aadac202 100644
--- a/lib/kernel/test/code_SUITE.erl
+++ b/lib/kernel/test/code_SUITE.erl
@@ -31,12 +31,17 @@
 	 where_is_file_cached/1, where_is_file_no_cache/1,
 	 purge_stacktrace/1, mult_lib_roots/1, bad_erl_libs/1,
 	 code_archive/1, code_archive2/1, on_load/1,
-	 on_load_embedded/1]).
+	 on_load_embedded/1, on_load_errors/1]).
 
 -export([init_per_testcase/2, fin_per_testcase/2, 
 	 init_per_suite/1, end_per_suite/1,
 	 sticky_compiler/1]).
 
+%% error_logger
+-export([init/1,
+	 handle_event/2, handle_call/2, handle_info/2,
+	 terminate/2]).
+
 all(suite) ->
     [set_path, get_path, add_path, add_paths, del_path,
      replace_path, load_file, load_abs, ensure_loaded,
@@ -47,7 +52,8 @@ all(suite) ->
      load_cached, start_node_with_cache, add_and_rehash,
      where_is_file_no_cache, where_is_file_cached,
      purge_stacktrace, mult_lib_roots, bad_erl_libs,
-     code_archive, code_archive2, on_load, on_load_embedded].
+     code_archive, code_archive2, on_load, on_load_embedded,
+     on_load_errors].
 
 init_per_suite(Config) ->
     %% The compiler will no longer create a Beam file if
@@ -671,6 +677,8 @@ check_funs({'$M_EXPR','$F_EXPR',2},
 check_funs({'$M_EXPR','$F_EXPR',1},
 	   [{lists,foreach,2},
 	    {hipe_unified_loader,patch_consts,3} | _]) -> 0;
+check_funs({'$M_EXPR',warning_msg,2},
+	   [{code_server,finish_on_load_report,2} | _]) -> 0;
 %% This is cheating! /raimo
 %% 
 %% check_funs(This = {M,_,_}, Path) ->
@@ -1229,6 +1237,81 @@ is_source_dir() ->
     filename:basename(code:lib_dir(kernel)) =:= "kernel" andalso
 	filename:basename(code:lib_dir(stdlib)) =:= "stdlib".
 
+on_load_errors(Config) when is_list(Config) ->
+    Master = on_load_error_test_case_process,
+    ?line register(Master, self()),
+
+    ?line Data = filename:join([?config(data_dir, Config),"on_load_errors"]),
+    ?line ok = file:set_cwd(Data),
+    ?line up_to_date = make:all([{d,'MASTER',Master}]),
+
+    ?line do_on_load_error(an_atom),
+
+    ?line error_logger:add_report_handler(?MODULE, self()),
+
+    ?line do_on_load_error({something,terrible,is,wrong}),
+    receive
+	Any1 ->
+	    ?line {_, "The on_load function"++_,
+		   [on_load_error,
+		    {something,terrible,is,wrong},_]} = Any1
+    end,
+
+    ?line do_on_load_error(fail),		%Cause exception.
+    receive
+	Any2 ->
+	    ?line {_, "The on_load function"++_,
+		   [on_load_error,{failed,[_|_]},_]} = Any2
+    end,
+
+    %% There should be no more messages.
+    receive
+	Unexpected ->
+	    ?line ?t:fail({unexpected,Unexpected})
+    after 10 ->
+	    ok
+    end,
+
+    ok.
+
+do_on_load_error(ReturnValue) ->
+    ?line {_,Ref} = spawn_monitor(fun() ->
+					  exit(on_load_error:main())
+				  end),
+    receive {on_load_error,ErrorPid} -> ok end,
+    ?line ErrorPid ! ReturnValue,
+    receive
+	{'DOWN',Ref,process,_,Exit} ->
+	    ?line {undef,[{on_load_error,main,[]}|_]} = Exit
+    end.
+
+%%-----------------------------------------------------------------
+%% error_logger handler.
+%% (Copied from stdlib/test/proc_lib_SUITE.erl.)
+%%-----------------------------------------------------------------
+init(Tester) ->
+    {ok, Tester}.
+
+handle_event({error, _GL, {emulator, _, _}}, Tester) ->
+    {ok, Tester};
+handle_event({error, _GL, Msg}, Tester) ->
+    Tester ! Msg,
+    {ok, Tester};
+handle_event(_Event, State) ->
+    {ok, State}.
+
+handle_info(_, State) ->
+    {ok, State}.
+
+handle_call(_Query, State) -> {ok, {error, bad_query}, State}.
+
+terminate(_Reason, State) ->
+    State.
+
+%%%
+%%% Common utility functions.
+%%%
+
 start_node(Name, Param) ->
     ?t:start_node(Name, slave, [{args, Param}]).
 
diff --git a/lib/kernel/test/code_SUITE_data/on_load/on_load_a.erl b/lib/kernel/test/code_SUITE_data/on_load/on_load_a.erl
index 660000df46..f6bcb6570b 100644
--- a/lib/kernel/test/code_SUITE_data/on_load/on_load_a.erl
+++ b/lib/kernel/test/code_SUITE_data/on_load/on_load_a.erl
@@ -13,7 +13,7 @@ on_load() ->
     LibDir = code:lib_dir(kernel),
 
     ?MASTER ! {?MODULE,LibDir},
-    true.
+    ok.
 
 data() ->
     [a|on_load_b:data()].
diff --git a/lib/kernel/test/code_SUITE_data/on_load/on_load_b.erl b/lib/kernel/test/code_SUITE_data/on_load/on_load_b.erl
index 5c4d676e2d..947cbd5bcd 100644
--- a/lib/kernel/test/code_SUITE_data/on_load/on_load_b.erl
+++ b/lib/kernel/test/code_SUITE_data/on_load/on_load_b.erl
@@ -6,7 +6,7 @@ on_load() ->
     ?MASTER ! {?MODULE,start},
     on_load_c:data(),
     ?MASTER ! {?MODULE,done},
-    true.
+    ok.
 
 data() ->
     [b|on_load_c:data()].
diff --git a/lib/kernel/test/code_SUITE_data/on_load/on_load_c.erl b/lib/kernel/test/code_SUITE_data/on_load/on_load_c.erl
index 4b2edbfb5a..6ab7f6402f 100644
--- a/lib/kernel/test/code_SUITE_data/on_load/on_load_c.erl
+++ b/lib/kernel/test/code_SUITE_data/on_load/on_load_c.erl
@@ -7,7 +7,7 @@ on_load() ->
     receive
 	go ->
 	    ?MASTER ! {?MODULE,done},
-	    true
+	    ok
     end.
 
 data() ->
diff --git a/lib/kernel/test/code_SUITE_data/on_load_app-1.0/src/on_load_embedded.erl b/lib/kernel/test/code_SUITE_data/on_load_app-1.0/src/on_load_embedded.erl
index bfc26864d5..a39332f81d 100644
--- a/lib/kernel/test/code_SUITE_data/on_load_app-1.0/src/on_load_embedded.erl
+++ b/lib/kernel/test/code_SUITE_data/on_load_app-1.0/src/on_load_embedded.erl
@@ -9,7 +9,7 @@ run_me() ->
 			  ok
 		  end
 	  end),
-    true.
+    ok.
 
 status() ->
     case whereis(everything_is_fine) of
diff --git a/lib/kernel/test/code_SUITE_data/on_load_errors/on_load_error.erl b/lib/kernel/test/code_SUITE_data/on_load_errors/on_load_error.erl
new file mode 100644
index 0000000000..0772050aeb
--- /dev/null
+++ b/lib/kernel/test/code_SUITE_data/on_load_errors/on_load_error.erl
@@ -0,0 +1,13 @@
+-module(on_load_error).
+-on_load(on_load/0).
+-export([main/0]).
+
+on_load() ->
+    ?MASTER ! {?MODULE,self()},
+    receive
+	fail -> erlang:error(failed);
+	Ret -> Ret
+    end.
+
+main() ->
+    ok.
diff --git a/system/doc/reference_manual/code_loading.xml b/system/doc/reference_manual/code_loading.xml
index 0e148298b1..f56e1ff408 100644
--- a/system/doc/reference_manual/code_loading.xml
+++ b/system/doc/reference_manual/code_loading.xml
@@ -121,8 +121,10 @@ loop() ->
     <title>Running a function when a module is loaded</title>
 
     <warning>
-    <p>This section describes an experimental feature introduced in R13B03.
-    There may be backward-incompatible changes in the feature in future releases.</p>
+    <p>This section describes an experimental feature that was
+      introduced in R13B03, and changed in a backwards-incompatible
+      way in R13B04. There may be more backward-incompatible changes
+      in future releases.</p>
     </warning>
 
     <p>The <c>-on_load()</c> directive names a function that should
@@ -133,25 +135,35 @@ loop() ->
 
     <p>It is not necessary to export the function. It will be called in a
     freshly spawned process (which will be terminated as soon as the function
-    returns). The function must return <c>true</c> if the module is to
-    be remained loaded and be callable, or <c>false</c> if the module
-    is to be unloaded. Returning any other value or generating an exception
-    will also cause the module to be unloaded.</p>
+    returns). The function must return <c>ok</c> if the module is to
+    be remained loaded and become callable, or any other value if the module
+    is to be unloaded. Generating an exception will also cause the
+    module to be unloaded. If the return value is not an atom,
+    a warning error report will be sent to the error logger.</p>
 
     <p>A process that calls any function in a module whose <c>on_load</c>
     function has not yet returned will be suspended until the <c>on_load</c>
     function has returned.</p>
 
+    <p>In embedded mode, all modules will be loaded first and then
+      will all on_load functions be called. The system will be
+      terminated unless all of the on_load functions return
+      <c>ok</c></p>.
+
     <p>Example:</p>
 
     <pre>
 -module(m).
--on_load(run_me/0).
+-on_load(load_my_nifs/0).
+
+load_my_nifs() ->
+    NifPath = ...,    %Set up the path to the NIF library.
+    Info = ...,       %Initialize the Info term
+    erlang:load_nif(NifPath, Info).</pre>
 
-run_me() ->
-    %% Do something with side effects here, for instance load a library
-    %% containing native-implemented functions.
-    true.</pre>
+    <p>If the call to <c>erlang:load_nif/2</c> fails, the module
+      will be unloaded and there will be warning report sent to
+      the error loader.</p>
 
   </section>