Prevent zip:zip_open/1,2 from leaking ports

The case was discovered where a parent process would exit before closing
the zip file. The result was that a port would be left open
indefinitely, as the small zip server would not detect this condition.

By comparison, the file module will close the associated port when the
parent exits for any reason. This change would make the zip module more
consistent with the semantics of similar modules.

This change is breaking for any callers expecting to pass the handle to
another process for processing (assuming it exits).

2 files changed, 57 insertions, 11 deletions

diff --git a/lib/stdlib/src/zip.erl b/lib/stdlib/src/zip.erl
index b768c6d0b9..cbd326ae92 100644
--- a/lib/stdlib/src/zip.erl
+++ b/lib/stdlib/src/zip.erl
@@ -1114,15 +1114,19 @@ local_file_header_from_info_method_name(#file_info{mtime = MTime},
 		       file_name_length = length(Name),
 		       extra_field_length = 0}.
 
+server_init(Parent) ->
+    %% we want to know if our parent dies
+    process_flag(trap_exit, true),
+    server_loop(Parent, not_open).
 
 %% small, simple, stupid zip-archive server
-server_loop(OpenZip) ->
+server_loop(Parent, OpenZip) ->
     receive
 	{From, {open, Archive, Options}} ->
 	    case openzip_open(Archive, Options) of
 		{ok, NewOpenZip} ->
 		    From ! {self(), {ok, self()}},
-		    server_loop(NewOpenZip);
+		    server_loop(Parent, NewOpenZip);
 		Error ->
 		    From ! {self(), Error}
 	    end;
@@ -1130,19 +1134,22 @@ server_loop(OpenZip) ->
 	    From ! {self(), openzip_close(OpenZip)};
 	{From, get} ->
 	    From ! {self(), openzip_get(OpenZip)},
-	    server_loop(OpenZip);
+	    server_loop(Parent, OpenZip);
 	{From, {get, FileName}} ->
 	    From ! {self(), openzip_get(FileName, OpenZip)},
-	    server_loop(OpenZip);
+	    server_loop(Parent, OpenZip);
 	{From, list_dir} ->
 	    From ! {self(), openzip_list_dir(OpenZip)},
-	    server_loop(OpenZip);
+	    server_loop(Parent, OpenZip);
 	{From, {list_dir, Opts}} ->
 	    From ! {self(), openzip_list_dir(OpenZip, Opts)},
-	    server_loop(OpenZip);
+	    server_loop(Parent, OpenZip);
 	{From, get_state} ->
 	    From ! {self(), OpenZip},
-	    server_loop(OpenZip);
+	    server_loop(Parent, OpenZip);
+        {'EXIT', Parent, Reason} ->
+            openzip_close(OpenZip),
+            exit({parent_died, Reason});
 	_ ->
 	    {error, bad_msg}
     end.
@@ -1162,8 +1169,9 @@ zip_open(Archive) -> zip_open(Archive, []).
       Reason :: term()).
 
 zip_open(Archive, Options) ->
-    Pid = spawn(fun() -> server_loop(not_open) end),
-    request(self(), Pid, {open, Archive, Options}).
+    Self = self(),
+    Pid = spawn_link(fun() -> server_init(Self) end),
+    request(Self, Pid, {open, Archive, Options}).
 
 -spec(zip_get(ZipHandle) -> {ok, [Result]} | {error, Reason} when
       ZipHandle :: pid(),
diff --git a/lib/stdlib/test/zip_SUITE.erl b/lib/stdlib/test/zip_SUITE.erl
index a57641ef62..d168a9d9bc 100644
--- a/lib/stdlib/test/zip_SUITE.erl
+++ b/lib/stdlib/test/zip_SUITE.erl
@@ -23,7 +23,7 @@
          bad_zip/1, unzip_from_binary/1, unzip_to_binary/1,
          zip_to_binary/1,
          unzip_options/1, zip_options/1, list_dir_options/1, aliases/1,
-         openzip_api/1, zip_api/1, unzip_jar/1,
+         openzip_api/1, zip_api/1, open_leak/1, unzip_jar/1,
          compress_control/1,
 	 foldl/1]).
 
@@ -38,7 +38,7 @@ all() ->
     [borderline, atomic, bad_zip, unzip_from_binary,
      unzip_to_binary, zip_to_binary, unzip_options,
      zip_options, list_dir_options, aliases, openzip_api,
-     zip_api, unzip_jar, compress_control, foldl].
+     zip_api, open_leak, unzip_jar, compress_control, foldl].
 
 groups() -> 
     [].
@@ -318,8 +318,46 @@ zip_api(Config) when is_list(Config) ->
     %% Clean up.
     delete_files([Names]),
 
+   ok.
+
+open_leak(doc) ->
+    ["Test that zip doesn't leak processes and ports where the "
+     "controlling process dies without closing an zip opened with "
+     "zip:zip_open/1."];
+open_leak(suite) -> [];
+open_leak(Config) when is_list(Config) ->
+    %% Create a zip archive
+    Zip = "zip.zip",
+    {ok, Zip} = zip:zip(Zip, [], []),
+
+    %% Open archive in a another process that dies immediately.
+    ZipSrv = spawn_zip(Zip, [memory]),
+
+    %% Expect the ZipSrv process to die soon after.
+    true = spawned_zip_dead(ZipSrv),
+
+    %% Clean up.
+    delete_files([Zip]),
+
     ok.
 
+spawn_zip(Zip, Options) ->
+    Self = self(),
+    spawn(fun() -> Self ! zip:zip_open(Zip, Options) end),
+    receive
+        {ok, ZipSrv} ->
+            ZipSrv
+    end.
+
+spawned_zip_dead(ZipSrv) ->
+    Ref = monitor(process, ZipSrv),
+    receive
+        {'DOWN', Ref, _, ZipSrv, _} ->
+            true
+    after 1000 ->
+            false
+    end.
+
 unzip_options(doc) ->
     ["Test options for unzip, only cwd and file_list currently"];
 unzip_options(suite) ->