Allow appup instruction delete_module module which is not loaded

The appup instruction 'delete_module' would cause a crash during
upgrade if the module to be deleted was not loaded. The reason was
that the release_handler tried to read the version number of the old
module after the code path had changed to point to the new version of
the application. Thus, if the module had not been loaded before the
upgrade, there would no longer be any such module in the path
(delete_module indicates that the module is deleted from the new
version of the application).

This is corrected by letting the release_handler read the old version
of the module only if the module is updated - not if it is
removed. And it is always read before the code path is changed.

10 files changed, 183 insertions, 14 deletions

diff --git a/lib/sasl/test/release_handler_SUITE.erl b/lib/sasl/test/release_handler_SUITE.erl
index d1939694ea..9c7733b7ec 100644
--- a/lib/sasl/test/release_handler_SUITE.erl
+++ b/lib/sasl/test/release_handler_SUITE.erl
@@ -55,7 +55,7 @@ win32_cases() ->
 
 %% Cases that can be run on all platforms
 cases() ->
-    [otp_2740, otp_2760, otp_5761, otp_9402, instructions, eval_appup].
+    [otp_2740, otp_2760, otp_5761, otp_9402, otp_9417, instructions, eval_appup].
 
 groups() ->
     [{release,[],
@@ -683,7 +683,7 @@ otp_9402(Conf) when is_list(Conf) ->
     %% Check path
     Dir1 = filename:join([LibDir, "a-1.1"]),
     Dir1 = rpc:call(Node, code, lib_dir, [a]),
-    ABeam = code:which(a),
+    ABeam = rpc:call(Node, code, which, [a]),
 
     %% Install second release, with no changed modules
     {ok, RelVsn2} =
@@ -696,7 +696,6 @@ otp_9402(Conf) when is_list(Conf) ->
     ok = rpc:call(Node, release_handler, install_file,
 		  [RelVsn2, filename:join(Rel2Dir, "sys.config")]),
 
-    %% Install RelVsn2
     {ok, RelVsn1, []} =
 	rpc:call(Node, release_handler, install_release, [RelVsn2]),
 
@@ -707,7 +706,7 @@ otp_9402(Conf) when is_list(Conf) ->
     true = filelib:is_regular(filename:join(APrivDir2,"file")),
 
     %% Just to make sure no modules have been re-loaded
-    ABeam = code:which(a),
+    ABeam = rpc:call(Node, code, which, [a]),
 
     %% Install RelVsn1 again
     {ok, _OtherVsn, []} =
@@ -719,11 +718,65 @@ otp_9402(Conf) when is_list(Conf) ->
     false = filelib:is_regular(filename:join(APrivDir1,"file")),
 
     %% Just to make sure no modules have been re-loaded
-    ABeam = code:which(a),
+    ABeam = rpc:call(Node, code, which, [a]),
 
     ok.
 
 
+%% When a module is deleted in an appup instruction, the upgrade
+%% failed if the module was not loaded.
+otp_9417(Conf) when is_list(Conf) ->
+    %% Set some paths
+    PrivDir = priv_dir(Conf),
+    Dir = filename:join(PrivDir,"otp_9417"),
+    LibDir = filename:join(?config(data_dir, Conf), "lib"),
+
+    %% Create the releases
+    Rel1 = create_and_install_fake_first_release(Dir,
+						 [{b,"1.0",LibDir}]),
+    Rel2 = create_fake_upgrade_release(Dir,
+				       "2",
+				       [{b,"2.0",LibDir}],
+				       {Rel1,Rel1,[LibDir]}),
+    Rel1Dir = filename:dirname(Rel1),
+    Rel2Dir = filename:dirname(Rel2),
+
+    %% Start a slave node
+    {ok, Node} = t_start_node(otp_9417, Rel1, filename:join(Rel1Dir,"sys.config")),
+
+    %% Check paths
+    Dir1 = filename:join([LibDir, "b-1.0"]),
+    Dir1 = rpc:call(Node, code, lib_dir, [b]),
+    BLibBeam = filename:join([Dir1,"ebin","b_lib.beam"]),
+    BLibBeam = rpc:call(Node,code,which,[b_lib]),
+    false = rpc:call(Node,code,is_loaded,[b_lib]),
+    false = rpc:call(Node,code,is_loaded,[b_server]),
+
+    %% Install second release, which removes b_lib module
+    {ok, RelVsn2} =
+	rpc:call(Node, release_handler, set_unpacked,
+		 [Rel2++".rel", [{b,"2.0",LibDir}]]),
+    ok = rpc:call(Node, release_handler, install_file,
+		  [RelVsn2, filename:join(Rel2Dir, "relup")]),
+    ok = rpc:call(Node, release_handler, install_file,
+		  [RelVsn2, filename:join(Rel2Dir, "start.boot")]),
+    ok = rpc:call(Node, release_handler, install_file,
+		  [RelVsn2, filename:join(Rel2Dir, "sys.config")]),
+
+    {ok, _RelVsn1, []} =
+	rpc:call(Node, release_handler, install_release, [RelVsn2]),
+
+    %% Check that the module does no longer exist
+    false = rpc:call(Node, code, is_loaded, [b_lib]),
+    non_existing = rpc:call(Node, code, which, [b_lib]),
+
+    %% And check some paths
+    Dir2 = filename:join([LibDir, "b-2.0"]),
+    Dir2 = rpc:call(Node, code, lib_dir, [b]),
+    BServerBeam = filename:join([Dir2,"ebin","b_server.beam"]),
+    {file,BServerBeam} = rpc:call(Node,code,is_loaded,[b_server]),
+    ok.
+
 %% Test upgrade and downgrade of applications
 eval_appup(Conf) when is_list(Conf) ->
 
diff --git a/lib/sasl/test/release_handler_SUITE_data/Makefile.src b/lib/sasl/test/release_handler_SUITE_data/Makefile.src
index abaa08f810..a12e526d2e 100644
--- a/lib/sasl/test/release_handler_SUITE_data/Makefile.src
+++ b/lib/sasl/test/release_handler_SUITE_data/Makefile.src
@@ -1,16 +1,19 @@
 EFLAGS=+debug_info
 
 P2B= \
-     P2B/a-2.0/ebin/a.beam \
-     P2B/a-2.0/ebin/a_sup.beam
+     P2B/a-2.0/ebin/a.@EMULATOR@ \
+     P2B/a-2.0/ebin/a_sup.@EMULATOR@
 
 LIB= \
-     lib/a-1.2/ebin/a.beam \
-     lib/a-1.2/ebin/a_sup.beam \
-     lib/a-1.1/ebin/a.beam \
-     lib/a-1.1/ebin/a_sup.beam \
-     lib/a-1.0/ebin/a.beam \
-     lib/a-1.0/ebin/a_sup.beam \
+     lib/a-1.2/ebin/a.@EMULATOR@ \
+     lib/a-1.2/ebin/a_sup.@EMULATOR@ \
+     lib/a-1.1/ebin/a.@EMULATOR@ \
+     lib/a-1.1/ebin/a_sup.@EMULATOR@ \
+     lib/a-1.0/ebin/a.@EMULATOR@ \
+     lib/a-1.0/ebin/a_sup.@EMULATOR@ \
+     lib/b-1.0/ebin/b_server.@EMULATOR@ \
+     lib/b-1.0/ebin/b_lib.@EMULATOR@ \
+     lib/b-2.0/ebin/b_server.@EMULATOR@
 
 APP= \
      app1_app2/lib1/app1-1.0/ebin/app1_sup.@EMULATOR@ \
@@ -64,6 +67,15 @@ lib/a-1.2/ebin/a.@EMULATOR@: lib/a-1.2/src/a.erl
 lib/a-1.2/ebin/a_sup.@EMULATOR@: lib/a-1.2/src/a_sup.erl
 	erlc $(EFLAGS) -olib/a-1.2/ebin lib/a-1.2/src/a_sup.erl
 
+lib/b-1.0/ebin/b_server.@EMULATOR@: lib/b-1.0/src/b_server.erl
+	erlc $(EFLAGS) -olib/b-1.0/ebin lib/b-1.0/src/b_server.erl
+lib/b-1.0/ebin/b_lib.@EMULATOR@: lib/b-1.0/src/b_lib.erl
+	erlc $(EFLAGS) -olib/b-1.0/ebin lib/b-1.0/src/b_lib.erl
+
+lib/b-2.0/ebin/b_server.@EMULATOR@: lib/b-2.0/src/b_server.erl
+	erlc $(EFLAGS) -olib/b-2.0/ebin lib/b-2.0/src/b_server.erl
+
+
 
 app1_app2/lib1/app1-1.0/ebin/app1_sup.@EMULATOR@: app1_app2/lib1/app1-1.0/src/app1_sup.erl
 	erlc $(EFLAGS) -oapp1_app2/lib1/app1-1.0/ebin app1_app2/lib1/app1-1.0/src/app1_sup.erl
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/README b/lib/sasl/test/release_handler_SUITE_data/lib/README
index e539580ac0..667d21d4cf 100644
--- a/lib/sasl/test/release_handler_SUITE_data/lib/README
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/README
@@ -6,4 +6,11 @@ can be upgraded to from a-1.0. Module a has changed
 
 a-1.2:
 can be upgraded to from a-1.1.
-No module have changed, but priv dir is added including one 'file'
\ No newline at end of file
+No module have changed, but priv dir is added including one 'file'
+
+b-1.0:
+start version, includes b_lib and b_server
+
+b-2.0:
+can be upgraded to from b-1.0.
+Removes b_lib and updates b_server
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/ebin/b.app b/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/ebin/b.app
new file mode 100644
index 0000000000..00347b2754
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/ebin/b.app
@@ -0,0 +1,7 @@
+%% -*- erlang -*-
+{application, b,
+ [{description, "B  CXC 138 12"},
+  {vsn, "1.0"},
+  {modules, [{b_server, 1},{b_lib, 1}]},
+  {registered, [b_server]},
+  {applications, [kernel, stdlib]}]}.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/src/b_lib.erl b/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/src/b_lib.erl
new file mode 100644
index 0000000000..7e8a308a5e
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/src/b_lib.erl
@@ -0,0 +1,3 @@
+-module(b_lib).
+-compile(export_all).
+foo() -> ok.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/src/b_server.erl b/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/src/b_server.erl
new file mode 100644
index 0000000000..e1a80a076f
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-1.0/src/b_server.erl
@@ -0,0 +1,37 @@
+-module(b_server).
+
+-behaviour(gen_server).
+
+%% API
+-export([start_link/0]).
+
+%% gen_server callbacks
+-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
+	 terminate/2, code_change/3]).
+
+-define(SERVER, ?MODULE).
+
+-record(state, {}).
+
+start_link() ->
+    gen_server:start_link({local, ?SERVER}, ?MODULE, [], []).
+
+init([]) ->
+    {ok, #state{}}.
+
+handle_call(_Request, _From, State) ->
+    Reply = ok,
+    {reply, Reply, State}.
+
+handle_cast(_Msg, State) ->
+    {noreply, State}.
+
+handle_info(_Info, State) ->
+    {noreply, State}.
+
+terminate(_Reason, _State) ->
+    ok.
+
+code_change(OldVsn, State, Extra) ->
+    file:write_file("/tmp/b_server",io_lib:format("~p~n",[{"1.0",OldVsn,Extra}])),
+    {ok, State}.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.app b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.app
new file mode 100644
index 0000000000..73c8e42b32
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.app
@@ -0,0 +1,7 @@
+%% -*- erlang -*-
+{application, b,
+ [{description, "B  CXC 138 12"},
+  {vsn, "2.0"},
+  {modules, [{b_server, 1}]},
+  {registered, [b_server]},
+  {applications, [kernel, stdlib]}]}.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup
new file mode 100644
index 0000000000..d261a37732
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/ebin/b.appup
@@ -0,0 +1,3 @@
+{"2.0",
+ [{"1.0",[{delete_module,b_lib}, {update,b_server,{advanced,[]}}]}],
+ [{"1.0",[{add_module,b_lib},{update,b_server,{advanced,[]}}]}]}.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/src/b_lib.erl b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/src/b_lib.erl
new file mode 100644
index 0000000000..7e8a308a5e
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/src/b_lib.erl
@@ -0,0 +1,3 @@
+-module(b_lib).
+-compile(export_all).
+foo() -> ok.
diff --git a/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/src/b_server.erl b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/src/b_server.erl
new file mode 100644
index 0000000000..f8bfbdaff7
--- /dev/null
+++ b/lib/sasl/test/release_handler_SUITE_data/lib/b-2.0/src/b_server.erl
@@ -0,0 +1,37 @@
+-module(b_server).
+
+-behaviour(gen_server).
+
+%% API
+-export([start_link/0]).
+
+%% gen_server callbacks
+-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
+	 terminate/2, code_change/3]).
+
+-define(SERVER, ?MODULE).
+
+-record(state, {}).
+
+start_link() ->
+    gen_server:start_link({local, ?SERVER}, ?MODULE, [], []).
+
+init([]) ->
+    {ok, #state{}}.
+
+handle_call(_Request, _From, State) ->
+    Reply = ok,
+    {reply, Reply, State}.
+
+handle_cast(_Msg, State) ->
+    {noreply, State}.
+
+handle_info(_Info, State) ->
+    {noreply, State}.
+
+terminate(_Reason, _State) ->
+    ok.
+
+code_change(OldVsn, State, Extra) ->
+    file:write_file("/tmp/b_server",io_lib:format("~p~n",[{"2.0",OldVsn,Extra}])),
+    {ok, State}.