Change the expected return value for on_load functions

An on_load function is supposed to return 'true' to indicate
that the module should be loaded, and 'false' if it should be
unloaded. But returning any other term, as well as causing an
exception, will also unload the module.

Since we don't like boolean values mixed with other values,
change the expected return value as follows:

* If 'ok' is returned, the module will remain loaded and become
  callable.

* If any other value is returned (or an exception is generated),
  the module will be unloaded. Also, if the returned value is
  not an atom, send a warning message to the error_logger
  (using error_logger:warning_msg/2).

The new interpretation of the return value means that an on_load
function can now directly return the return value from
erlang:load_nif/2.

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 ->

lib/kernel/src/code_server.erl

@@ -1479,20 +1479,37 @@ 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}
 	  end,
     [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.
 %% -------------------------------------------------------

lib/kernel/test/code_SUITE.erl

@@ -671,6 +671,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) ->

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()].

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()].

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() ->

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

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>