From 71b324c26cd191a6b4e60e6be8e35487723bcb14 Mon Sep 17 00:00:00 2001 From: Diana Corbacho Date: Wed, 17 May 2017 17:46:33 +0100 Subject: [PATCH 1/5] Stop emitting stats before hibernate on gen_server2 rabbbitmq-common#196 [#145623415] --- deps/rabbit_common/src/gen_server2.erl | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/deps/rabbit_common/src/gen_server2.erl b/deps/rabbit_common/src/gen_server2.erl index 4fdcc564cf..4209f4a06b 100644 --- a/deps/rabbit_common/src/gen_server2.erl +++ b/deps/rabbit_common/src/gen_server2.erl @@ -710,6 +710,7 @@ hibernate(GS2State = #gs2_state { timeout_state = TimeoutState }) -> pre_hibernate(GS2State = #gs2_state { state = State, mod = Mod }) -> + rabbit_event:stop_stats_timer(GS2State, #gs2_state.timer), case erlang:function_exported(Mod, handle_pre_hibernate, 1) of true -> case catch Mod:handle_pre_hibernate(State) of @@ -723,16 +724,17 @@ pre_hibernate(GS2State = #gs2_state { state = State, end. post_hibernate(GS2State = #gs2_state { state = State, - mod = Mod }) -> + mod = Mod, + init_stats_fun = InitStatsFun }) -> case erlang:function_exported(Mod, handle_post_hibernate, 1) of true -> case catch Mod:handle_post_hibernate(State) of {noreply, NState} -> - process_next_msg(GS2State #gs2_state { state = NState, - time = infinity }); + process_next_msg(InitStatsFun(GS2State #gs2_state { state = NState, + time = infinity })); {noreply, NState, Time} -> - process_next_msg(GS2State #gs2_state { state = NState, - time = Time }); + process_next_msg(InitStatsFun(GS2State #gs2_state { state = NState, + time = Time })); Reply -> handle_common_termination(Reply, post_hibernate, GS2State) end; @@ -743,7 +745,7 @@ post_hibernate(GS2State = #gs2_state { state = State, %% still set to hibernate, iff that msg is the very msg %% that woke us up (or the first msg we receive after %% waking up). - process_next_msg(GS2State #gs2_state { time = hibernate }) + process_next_msg(InitStatsFun(GS2State #gs2_state { time = hibernate })) end. adjust_timeout_state(SleptAt, AwokeAt, {backoff, CurrentTO, MinimumTO, From e4327950ad1cd03e9b32ee5b79aae4fcba74722f Mon Sep 17 00:00:00 2001 From: Diana Corbacho Date: Thu, 18 May 2017 09:54:44 +0100 Subject: [PATCH 2/5] Stop stats only if they have been initialised * Detected on unit tests, it should never happen on a rabbit node --- deps/rabbit_common/src/gen_server2.erl | 30 +++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/deps/rabbit_common/src/gen_server2.erl b/deps/rabbit_common/src/gen_server2.erl index 4209f4a06b..ae1284cd74 100644 --- a/deps/rabbit_common/src/gen_server2.erl +++ b/deps/rabbit_common/src/gen_server2.erl @@ -708,9 +708,9 @@ hibernate(GS2State = #gs2_state { timeout_state = TimeoutState }) -> proc_lib:hibernate(?MODULE, wake_hib, [GS2State #gs2_state { timeout_state = TS }]). -pre_hibernate(GS2State = #gs2_state { state = State, - mod = Mod }) -> - rabbit_event:stop_stats_timer(GS2State, #gs2_state.timer), +pre_hibernate(GS2State0 = #gs2_state { state = State, + mod = Mod }) -> + GS2State = maybe_stop_stats(GS2State0), case erlang:function_exported(Mod, handle_pre_hibernate, 1) of true -> case catch Mod:handle_pre_hibernate(State) of @@ -723,18 +723,19 @@ pre_hibernate(GS2State = #gs2_state { state = State, hibernate(GS2State) end. -post_hibernate(GS2State = #gs2_state { state = State, - mod = Mod, - init_stats_fun = InitStatsFun }) -> +post_hibernate(GS2State0 = #gs2_state { state = State, + mod = Mod, + init_stats_fun = InitStatsFun }) -> + GS2State = InitStatsFun(GS2State0), case erlang:function_exported(Mod, handle_post_hibernate, 1) of true -> case catch Mod:handle_post_hibernate(State) of {noreply, NState} -> - process_next_msg(InitStatsFun(GS2State #gs2_state { state = NState, - time = infinity })); + process_next_msg(GS2State #gs2_state { state = NState, + time = infinity }); {noreply, NState, Time} -> - process_next_msg(InitStatsFun(GS2State #gs2_state { state = NState, - time = Time })); + process_next_msg(GS2State #gs2_state { state = NState, + time = Time }); Reply -> handle_common_termination(Reply, post_hibernate, GS2State) end; @@ -745,7 +746,7 @@ post_hibernate(GS2State = #gs2_state { state = State, %% still set to hibernate, iff that msg is the very msg %% that woke us up (or the first msg we receive after %% waking up). - process_next_msg(InitStatsFun(GS2State #gs2_state { time = hibernate })) + process_next_msg(GS2State #gs2_state { time = hibernate }) end. adjust_timeout_state(SleptAt, AwokeAt, {backoff, CurrentTO, MinimumTO, @@ -1383,5 +1384,10 @@ emit_stats(GS2State = #gs2_state{queue = Queue}) -> #gs2_state.timer, emit_gen_server2_stats). stop_stats(GS2State) -> - _ = rabbit_event:stop_stats_timer(GS2State, #gs2_state.timer), + maybe_stop_stats(GS2State), rabbit_core_metrics:gen_server2_deleted(self()). + +maybe_stop_stats(#gs2_state{timer = undefined} = GS2State) -> + GS2State; +maybe_stop_stats(GS2State) -> + rabbit_event:stop_stats_timer(GS2State, #gs2_state.timer). From 1de65585f26409f4403f06255fe54b4f34672653 Mon Sep 17 00:00:00 2001 From: Daniil Fedotov Date: Fri, 12 May 2017 11:20:56 +0100 Subject: [PATCH 3/5] Change the way we filter supported ciphers and hashes. To avoid hitting a bug in 20.0-rc1 when aes_gcp cipher was duplicated. --- deps/rabbit_common/src/rabbit_pbe.erl | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/deps/rabbit_common/src/rabbit_pbe.erl b/deps/rabbit_common/src/rabbit_pbe.erl index 59a2ff9c38..d5614152b8 100644 --- a/deps/rabbit_common/src/rabbit_pbe.erl +++ b/deps/rabbit_common/src/rabbit_pbe.erl @@ -23,12 +23,20 @@ %% Supported ciphers and hashes supported_ciphers() -> - proplists:get_value(ciphers, crypto:supports()) - -- [aes_ctr, aes_ecb, des_ecb, blowfish_ecb, rc4, aes_gcm]. + NotSupportedByUs = [aes_ctr, aes_ecb, des_ecb, blowfish_ecb, rc4, aes_gcm], + SupportedByCrypto = proplists:get_value(ciphers, crypto:supports()), + lists:filter(fun(Cipher) -> + not lists:member(Cipher, NotSupportedByUs) + end, + SupportedByCrypto). supported_hashes() -> - proplists:get_value(hashs, crypto:supports()) - -- [md4, ripemd160]. + NotSupportedByUs = [md4, ripemd160], + SupportedByCrypto = proplists:get_value(hashs, crypto:supports()), + lists:filter(fun(Hash) -> + not lists:member(Hash, NotSupportedByUs) + end, + SupportedByCrypto). %% Default encryption parameters (keep those in sync with rabbit.app.src) default_cipher() -> From 968fc8a5783ab89e1e29d4ae54a5a786d43b2715 Mon Sep 17 00:00:00 2001 From: Daniil Fedotov Date: Thu, 11 May 2017 15:54:43 +0100 Subject: [PATCH 4/5] Do not assume Node part format in Pid binary. Pid binary format has changed in OTP-20.0. We can still get the node using node/1, so we can skip the node part when decomposing a pid. Added unit tests for decomposing and composing a pid and changing the node. --- deps/rabbit_common/src/rabbit_misc.erl | 12 +++++++++--- deps/rabbit_common/test/unit_SUITE.erl | 12 +++++++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/deps/rabbit_common/src/rabbit_misc.erl b/deps/rabbit_common/src/rabbit_misc.erl index c0c7568641..c0c9597bc9 100644 --- a/deps/rabbit_common/src/rabbit_misc.erl +++ b/deps/rabbit_common/src/rabbit_misc.erl @@ -19,6 +19,10 @@ -include("rabbit_framing.hrl"). -include("rabbit_misc.hrl"). +-ifdef(TEST). +-export([decompose_pid/1, compose_pid/4]). +-endif. + -export([method_record_type/1, polite_pause/0, polite_pause/1]). -export([die/1, frame_error/2, amqp_error/4, quit/1, protocol_error/3, protocol_error/4, protocol_error/1]). @@ -732,9 +736,11 @@ node_to_fake_pid(Node) -> decompose_pid(Pid) when is_pid(Pid) -> %% see http://erlang.org/doc/apps/erts/erl_ext_dist.html (8.10 and %% 8.7) - <<131,103,100,NodeLen:16,NodeBin:NodeLen/binary,Id:32,Ser:32,Cre:8>> - = term_to_binary(Pid), - Node = binary_to_term(<<131,100,NodeLen:16,NodeBin:NodeLen/binary>>), + Node = node(Pid), + BinPid = term_to_binary(Pid), + ByteSize = byte_size(BinPid), + NodeByteSize = (ByteSize - 11), + <<131, 103, _NodePrefix:NodeByteSize/binary, Id:32, Ser:32, Cre:8>> = BinPid, {Node, Cre, Id, Ser}. compose_pid(Node, Cre, Id, Ser) -> diff --git a/deps/rabbit_common/test/unit_SUITE.erl b/deps/rabbit_common/test/unit_SUITE.erl index b0436a5530..043b205cda 100644 --- a/deps/rabbit_common/test/unit_SUITE.erl +++ b/deps/rabbit_common/test/unit_SUITE.erl @@ -33,13 +33,23 @@ groups() -> encrypt_decrypt_term, version_equivalence, version_minor_equivalence_properties, - version_comparison + version_comparison, + pid_decompose_compose ]} ]. init_per_group(_, Config) -> Config. end_per_group(_, Config) -> Config. +pid_decompose_compose(_Config) -> + Pid = self(), + {Node, Cre, Id, Ser} = rabbit_misc:decompose_pid(Pid), + Node = node(Pid), + Pid = rabbit_misc:compose_pid(Node, Cre, Id, Ser), + OtherNode = 'some_node@localhost', + PidOnOtherNode = rabbit_misc:pid_change_node(Pid, OtherNode), + {OtherNode, Cre, Id, Ser} = rabbit_misc:decompose_pid(PidOnOtherNode). + encrypt_decrypt(_Config) -> %% Take all available block ciphers. Hashes = rabbit_pbe:supported_hashes(), From 5f46c33dc731cc5c1a93507813548d4865911f8a Mon Sep 17 00:00:00 2001 From: Daniil Fedotov Date: Fri, 19 May 2017 12:30:27 +0100 Subject: [PATCH 5/5] Add nowarn_export_all to erlang compile options export_all is used in rabbit_upgrades, but causes a warning in OTP-20 --- deps/rabbit_common/mk/rabbitmq-build.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/deps/rabbit_common/mk/rabbitmq-build.mk b/deps/rabbit_common/mk/rabbitmq-build.mk index adf710cedf..870b13bdc4 100644 --- a/deps/rabbit_common/mk/rabbitmq-build.mk +++ b/deps/rabbit_common/mk/rabbitmq-build.mk @@ -3,6 +3,7 @@ # -------------------------------------------------------------------- TEST_ERLC_OPTS += +nowarn_export_all +ERLC_OPTS += +nowarn_export_all define compare_version $(shell awk 'BEGIN {