diff options
author | Rickard Green <rickard@erlang.org> | 2023-04-29 01:00:45 +0200 |
---|---|---|
committer | Rickard Green <rickard@erlang.org> | 2023-04-29 01:00:45 +0200 |
commit | 789bbbb26552128df5af02c2a3f39e96b2eb8f0f (patch) | |
tree | b376561264c4d6e4213cb3ca5716e59a04c395dd | |
parent | 0863bd30aabd035c83158c78046c5ffda16127e1 (diff) | |
parent | 330b119be68f95f6f8fbdef8bf16bba314783973 (diff) | |
download | erlang-789bbbb26552128df5af02c2a3f39e96b2eb8f0f.tar.gz |
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
-rw-r--r-- | erts/emulator/beam/dist.c | 13 | ||||
-rw-r--r-- | erts/emulator/beam/erl_node_tables.c | 12 | ||||
-rw-r--r-- | erts/emulator/beam/erl_node_tables.h | 8 | ||||
-rw-r--r-- | erts/emulator/test/distribution_SUITE.erl | 123 |
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). |