From 656420b64c9207ef5fc25c107b5f4a457b6359bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20H=C3=B6gberg?= <john@erlang.org>
Date: Thu, 9 May 2019 15:07:48 +0200
Subject: cover: Fix register/2 race on startup

---
 lib/tools/src/cover.erl        | 42 ++++++++++++++++++++++++++++--------------
 lib/tools/test/cover_SUITE.erl | 29 ++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/lib/tools/src/cover.erl b/lib/tools/src/cover.erl
index 4e64d7aa4e..3565f8ba70 100644
--- a/lib/tools/src/cover.erl
+++ b/lib/tools/src/cover.erl
@@ -167,6 +167,8 @@ start() ->
 		receive 
 		    {?SERVER,started} -> 
 			{ok,Pid};
+		    {?SERVER,{error,Error}} -> 
+			{error,Error};
 		    {'DOWN', Ref, _Type, _Object, Info} -> 
 			{error,Info}
 		end,
@@ -628,21 +630,33 @@ remote_reply(MainNode,Reply) ->
 %%%----------------------------------------------------------------------
 
 init_main(Starter) ->
-    register(?SERVER,self()),
-    %% Having write concurrancy here gives a 40% performance boost
-    %% when collect/1 is called. 
-    ?COVER_TABLE = ets:new(?COVER_TABLE, [set, public, named_table,
-                                          {write_concurrency, true}]),
-    ?COVER_CLAUSE_TABLE = ets:new(?COVER_CLAUSE_TABLE, [set, public,
+    try register(?SERVER,self()) of
+            true ->
+        %% Having write concurrancy here gives a 40% performance boost
+        %% when collect/1 is called. 
+        ?COVER_TABLE = ets:new(?COVER_TABLE, [set, public, named_table,
+                                              {write_concurrency, true}]),
+        ?COVER_CLAUSE_TABLE = ets:new(?COVER_CLAUSE_TABLE, [set, public,
+                                                            named_table]),
+        ?BINARY_TABLE = ets:new(?BINARY_TABLE, [set, public, named_table]),
+        ?COLLECTION_TABLE = ets:new(?COLLECTION_TABLE, [set, public,
                                                         named_table]),
-    ?BINARY_TABLE = ets:new(?BINARY_TABLE, [set, public, named_table]),
-    ?COLLECTION_TABLE = ets:new(?COLLECTION_TABLE, [set, public,
-                                                    named_table]),
-    ?COLLECTION_CLAUSE_TABLE = ets:new(?COLLECTION_CLAUSE_TABLE, [set, public,
-                                                                  named_table]),
-    ok = net_kernel:monitor_nodes(true),
-    Starter ! {?SERVER,started},
-    main_process_loop(#main_state{}).
+        ?COLLECTION_CLAUSE_TABLE = ets:new(?COLLECTION_CLAUSE_TABLE,
+                                           [set, public, named_table]),
+        ok = net_kernel:monitor_nodes(true),
+        Starter ! {?SERVER,started},
+        main_process_loop(#main_state{})
+    catch
+        error:badarg ->
+            %% The server's already registered; either report that it's already
+            %% started or try again if it died before we could find its pid.
+            case whereis(?SERVER) of
+                undefined ->
+                    init_main(Starter);
+                Pid ->
+                    Starter ! {?SERVER, {error, {already_started, Pid}}}
+            end
+    end.
 
 main_process_loop(State) ->
     receive
diff --git a/lib/tools/test/cover_SUITE.erl b/lib/tools/test/cover_SUITE.erl
index 161b0105b9..806297abdd 100644
--- a/lib/tools/test/cover_SUITE.erl
+++ b/lib/tools/test/cover_SUITE.erl
@@ -35,7 +35,8 @@ all() ->
                  distribution, reconnect, die_and_reconnect,
                  dont_reconnect_after_stop, stop_node_after_disconnect,
                  export_import, otp_5031, otp_6115,
-                 otp_8270, otp_10979_hanging_node, otp_14817],
+                 otp_8270, otp_10979_hanging_node, otp_14817,
+                 startup_race],
     case whereis(cover_server) of
         undefined ->
             [coverage,StartStop ++ NoStartStop];
@@ -1742,6 +1743,32 @@ otp_13289(Config) ->
     ok = file:delete(File),
     ok.
 
+%% ERL-943; We should not crash on startup when multiple servers race to
+%% register the server name.
+startup_race(Config) when is_list(Config) ->
+    PidRefs = [spawn_monitor(fun() ->
+                                     case cover:start() of
+                                         {error, {already_started, _Pid}} ->
+                                             ok;
+                                         {ok, _Pid} ->
+                                             ok
+                                     end
+                             end) || _<- lists:seq(1,8)],
+    startup_race_1(PidRefs).
+
+startup_race_1([{Pid, Ref} | PidRefs]) ->
+    receive
+        {'DOWN', Ref, process, Pid, normal} ->
+            startup_race_1(PidRefs);
+        {'DOWN', Ref, process, Pid, _Other} ->
+            ct:fail("Cover server crashed on startup.")
+    after 5000 ->
+            ct:fail("Timed out.")
+    end;
+startup_race_1([]) ->
+    cover:stop(),
+    ok.
+
 %%--Auxiliary------------------------------------------------------------
 
 analyse_expr(Expr, Config) ->
-- 
cgit v1.2.3