Merge branch 'rickard/creation-fix/23.3.4/OTP-18570' into rickard/creation-fix/24.3.4/OTP-18570

* rickard/creation-fix/23.3.4/OTP-18570:
  [erts] ensure no mix of external and internal identifiers

4 files changed, 154 insertions, 2 deletions

diff --git a/erts/emulator/beam/dist.c b/erts/emulator/beam/dist.c
index 0967b94809..1d6668facc 100644
--- a/erts/emulator/beam/dist.c
+++ b/erts/emulator/beam/dist.c
@@ -4685,6 +4685,19 @@ BIF_RETTYPE setnode_2(BIF_ALIST_2)
     success = (!ERTS_PROC_IS_EXITING(net_kernel)
                & !ERTS_PROC_GET_DIST_ENTRY(net_kernel));
     if (success) {
+        /*
+         * Ensure we don't use a nodename-creation pair with
+         * external identifiers existing in the system.
+         */
+        while (!0) {
+            ErlNode *nep;
+            if (creation < 4)
+                creation = 4;
+            nep = erts_find_node(BIF_ARG_1, creation);
+            if (!nep || erts_node_refc(nep) == 0)
+                break;
+            creation++;
+        }
         inc_no_nodes();
         erts_set_this_node(BIF_ARG_1, (Uint32) creation);
         erts_this_dist_entry->creation = creation;
diff --git a/erts/emulator/beam/erl_node_tables.c b/erts/emulator/beam/erl_node_tables.c
index 912caf5ace..81decbe11b 100644
--- a/erts/emulator/beam/erl_node_tables.c
+++ b/erts/emulator/beam/erl_node_tables.c
@@ -884,6 +884,18 @@ erts_node_table_info(fmtfn_t to, void *to_arg)
 	erts_rwmtx_runlock(&erts_node_table_rwmtx);
 }
 
+ErlNode *erts_find_node(Eterm sysname, Uint32 creation)
+{
+    ErlNode *res;
+    ErlNode ne;
+    ne.sysname = sysname;
+    ne.creation = creation;
+
+    erts_rwmtx_rlock(&erts_node_table_rwmtx);
+    res = hash_get(&erts_node_table, (void *) &ne);
+    erts_rwmtx_runlock(&erts_node_table_rwmtx);
+    return res;
+}
 
 ErlNode *erts_find_or_insert_node(Eterm sysname, Uint32 creation, Eterm book)
 {
diff --git a/erts/emulator/beam/erl_node_tables.h b/erts/emulator/beam/erl_node_tables.h
index 937bfb1d9e..c3ee2eaa36 100644
--- a/erts/emulator/beam/erl_node_tables.h
+++ b/erts/emulator/beam/erl_node_tables.h
@@ -258,6 +258,7 @@ void erts_set_dist_entry_not_connected(DistEntry *);
 void erts_set_dist_entry_pending(DistEntry *);
 void erts_set_dist_entry_connected(DistEntry *, Eterm, Uint64);
 ErlNode *erts_find_or_insert_node(Eterm, Uint32, Eterm);
+ErlNode *erts_find_node(Eterm, Uint32);
 void erts_schedule_delete_node(ErlNode *);
 void erts_set_this_node(Eterm, Uint32);
 Uint erts_node_table_size(void);
@@ -285,6 +286,7 @@ ERTS_GLB_INLINE void erts_deref_node_entry__(ErlNode *np, Eterm term, char *file
 ERTS_GLB_INLINE erts_aint_t erts_ref_node_entry(ErlNode *np, int min_val, Eterm term);
 ERTS_GLB_INLINE void erts_deref_node_entry(ErlNode *np, Eterm term);
 #endif
+ERTS_GLB_INLINE erts_aint_t erts_node_refc(ErlNode *np);
 ERTS_GLB_INLINE void erts_de_rlock(DistEntry *dep);
 ERTS_GLB_INLINE void erts_de_runlock(DistEntry *dep);
 ERTS_GLB_INLINE void erts_de_rwlock(DistEntry *dep);
@@ -335,6 +337,12 @@ erts_deref_node_entry(ErlNode *np, Eterm term)
 	erts_schedule_delete_node(np);
 }
 
+ERTS_GLB_INLINE erts_aint_t
+erts_node_refc(ErlNode *np)
+{
+    return erts_refc_read(&np->refc, 0);
+}
+
 #endif
 
 ERTS_GLB_INLINE void
diff --git a/erts/emulator/test/distribution_SUITE.erl b/erts/emulator/test/distribution_SUITE.erl
index 23a6018e69..8bd3147d68 100644
--- a/erts/emulator/test/distribution_SUITE.erl
+++ b/erts/emulator/test/distribution_SUITE.erl
@@ -77,7 +77,9 @@
          system_limit/1,
          hopefull_data_encoding/1,
          hopefull_export_fun_bug/1,
-         huge_iovec/1]).
+         huge_iovec/1,
+         creation_selection/1,
+         creation_selection_test/1]).
 
 %% Internal exports.
 -export([sender/3, receiver2/2, dummy_waiter/0, dead_process/0,
@@ -110,7 +112,7 @@ all() ->
      dist_entry_refc_race,
      start_epmd_false, no_epmd, epmd_module, system_limit,
      hopefull_data_encoding, hopefull_export_fun_bug,
-     huge_iovec].
+     huge_iovec, creation_selection].
 
 groups() ->
     [{bulk_send, [], [bulk_send_small, bulk_send_big, bulk_send_bigbig]},
@@ -3004,6 +3006,120 @@ mk_rand_bin(0, Data) ->
 mk_rand_bin(N, Data) ->
     mk_rand_bin(N-1, [rand:uniform(256) - 1 | Data]).
 
+creation_selection(Config) when is_list(Config) ->
+    register(creation_selection_test_supervisor, self()),
+    Name = atom_to_list(?FUNCTION_NAME) ++ "-"
+        ++ integer_to_list(erlang:system_time()),
+    Host = hostname(),
+    Cmd = lists:append(
+            [ct:get_progname(),
+             " -noshell",
+             " -setcookie ", atom_to_list(erlang:get_cookie()),
+             " -pa ", filename:dirname(code:which(?MODULE)),
+             " -s ", atom_to_list(?MODULE), " ",
+             " creation_selection_test ", atom_to_list(node()), " ",
+             atom_to_list(net_kernel:longnames()), " ", Name, " ", Host]),
+    ct:pal("Node command: ~p~n", [Cmd]),
+    Port = open_port({spawn, Cmd}, [exit_status]),
+    Node = list_to_atom(lists:append([Name, "@", Host])),
+    ok = receive_creation_selection_info(Port, Node).
+
+receive_creation_selection_info(Port, Node) ->
+    receive
+        {creation_selection_test, Node, Creations, InvalidCreation,
+         ClashResolvedCreation} = Msg ->
+            ct:log("Test result: ~p~n", [Msg]),
+            %% Verify that creation values are created as expected. The
+            %% list of creations is in reverse start order...
+            MaxC = (1 bsl 32) - 1,
+            MinC = 4,
+            StartOrderCreations = lists:reverse(Creations),
+            InvalidCreation = lists:foldl(fun (C, C) when is_integer(C),
+                                                          MinC =< C,
+                                                          C =< MaxC ->
+                                                  %% Return next expected
+                                                  %% creation...
+                                                  if C == MaxC -> MinC;
+                                                     true -> C+1
+                                                  end
+                                          end,
+                                          hd(StartOrderCreations),
+                                          StartOrderCreations),
+            false = lists:member(ClashResolvedCreation, [InvalidCreation
+                                                        | Creations]),
+            receive
+                {Port, {exit_status, 0}} ->
+                    Port ! {self(), close},
+                    ok;
+                {Port, {exit_status, EStat}} ->
+                    ct:fail({"node exited abnormally: ", EStat})
+            end;
+        {Port, {exit_status, EStat}} ->
+            ct:fail({"node prematurely exited: ", EStat});
+        {Port, {data, Data}} ->
+            ct:log("~ts", [Data]),
+            receive_creation_selection_info(Port, Node)
+    end,
+    ok.
+
+creation_selection_test([TestSupNode, LongNames, Name, Host]) ->
+    try
+        StartArgs = [Name,
+                     case LongNames of
+                         true -> longnames;
+                         false -> shortnames
+                     end],
+        Node = list_to_atom(lists:append([atom_to_list(Name),
+                                          "@", atom_to_list(Host)])),
+        GoDistributed = fun (F) ->
+                                {ok, _} = net_kernel:start(StartArgs),
+                                Node = node(),
+                                Creation = erlang:system_info(creation),
+                                _ = F(Creation),
+                                net_kernel:stop(),
+                                Creation
+                        end,
+        %% We start multiple times to verify that the creation values
+        %% we get from epmd are delivered in sequence. This is a
+        %% must for the test case such as it is written now, but can be
+        %% changed. If changed, this test case must be updated...
+        {Creations,
+         LastCreation} = lists:foldl(fun (_, {Cs, _LC}) ->
+                                             CFun = fun (X) -> X end,
+                                             C = GoDistributed(CFun),
+                                             {[C|Cs], C}
+                                     end, {[], 0}, lists:seq(1, 5)),
+        %% We create a pid with the creation that epmd will offer us the next
+        %% time we start the distribution and then start the distribution
+        %% once more. The node should avoid this creation, since this would
+        %% cause external identifiers in the system with same
+        %% nodename/creation pair as used by the local node, which in turn
+        %% would cause these identifers not to work as expected. That is, the
+        %% node should silently reject this creation and chose another one when
+        %% starting the distribution.
+        InvalidCreation = LastCreation+1,
+        Pid = erts_test_utils:mk_ext_pid({Node, InvalidCreation}, 4711, 0),
+        true = erts_debug:size(Pid) > 0, %% External pid
+        ResultFun = fun (ClashResolvedCreation) ->
+                            pong = net_adm:ping(TestSupNode),
+                            Msg = {creation_selection_test, node(), Creations,
+                                   InvalidCreation, ClashResolvedCreation},
+                            {creation_selection_test_supervisor, TestSupNode}
+                                ! Msg,
+                            %% Wait a bit so the message have time to get
+                            %% through before we take down the distribution...
+                            receive after 500 -> ok end
+                    end,
+        _ = GoDistributed(ResultFun),
+        %% Ensure Pid is not garbage collected before starting the
+        %% distribution...
+        _ = id(Pid),
+        erlang:halt(0)
+    catch
+        Class:Reason:StackTrace ->
+            erlang:display({Class, Reason, StackTrace}),
+            erlang:halt(17)
+    end.
 
 %% Try provoke DistEntry refc bugs (OTP-17513).
 dist_entry_refc_race(_Config) ->
@@ -3049,6 +3165,9 @@ derr_sender(Main, Nodes) ->
 
 %%% Utilities
 
+id(X) ->
+    X.
+
 timestamp() ->
     erlang:monotonic_time(millisecond).