Don't be so intrusive into gm.erl.

3 files changed, 63 insertions, 76 deletions

diff --git a/Makefile b/Makefile
index 127fa2fe..7bfdf44e 100644
--- a/Makefile
+++ b/Makefile
@@ -56,6 +56,12 @@ endif
 #other args: +native +"{hipe,[o3,verbose]}" -Ddebug=true +debug_info +no_strict_record_tests
 ERLC_OPTS=-I $(INCLUDE_DIR) -o $(EBIN_DIR) -Wall -v +debug_info $(call boolean_macro,$(USE_SPECS),use_specs) $(call boolean_macro,$(USE_PROPER_QC),use_proper_qc)
 
+ifdef INSTRUMENT_FOR_QC
+ERLC_OPTS += -DINSTR_MOD=gm_qc
+else
+ERLC_OPTS += -DINSTR_MOD=gen_server2
+endif
+
 include version.mk
 
 PLUGINS_SRC_DIR?=$(shell [ -d "plugins-src" ] && echo "plugins-src" || echo )
diff --git a/src/gm.erl b/src/gm.erl
index 4065b171..e8d56e8c 100644
--- a/src/gm.erl
+++ b/src/gm.erl
@@ -382,8 +382,7 @@
 
 -behaviour(gen_server2).
 
--export([create_tables/0, start_link/4, start_link/6,
-         leave/1, broadcast/2, broadcast/3,
+-export([create_tables/0, start_link/4, leave/1, broadcast/2, broadcast/3,
          confirmed_broadcast/2, info/1, validate_members/2, forget_group/1]).
 
 -export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2,
@@ -418,9 +417,7 @@
           broadcast_buffer,
           broadcast_buffer_sz,
           broadcast_timer,
-          txn_executor,
-          call,
-          cast
+          txn_executor
         }).
 
 -record(gm_group, { name, version, members }).
@@ -522,14 +519,7 @@ table_definitions() ->
     [{Name, [?TABLE_MATCH | Attributes]}].
 
 start_link(GroupName, Module, Args, TxnFun) ->
-    start_link(GroupName, Module, Args, TxnFun,
-               fun gen_server2:call/3, fun gen_server2:cast/2).
-
-%% For testing we can instrument certain "interesting" cases where we
-%% call and cast to other GMs.
-start_link(GroupName, Module, Args, TxnFun, Call, Cast) ->
-    gen_server2:start_link(
-      ?MODULE, [GroupName, Module, Args, TxnFun, Call, Cast], []).
+    gen_server2:start_link(?MODULE, [GroupName, Module, Args, TxnFun], []).
 
 leave(Server) ->
     gen_server2:cast(Server, leave).
@@ -555,7 +545,7 @@ forget_group(GroupName) ->
                      end),
     ok.
 
-init([GroupName, Module, Args, TxnFun, Call, Cast]) ->
+init([GroupName, Module, Args, TxnFun]) ->
     put(process_name, {?MODULE, GroupName}),
     {MegaSecs, Secs, MicroSecs} = now(),
     random:seed(MegaSecs, Secs, MicroSecs),
@@ -574,9 +564,7 @@ init([GroupName, Module, Args, TxnFun, Call, Cast]) ->
                   broadcast_buffer    = [],
                   broadcast_buffer_sz = 0,
                   broadcast_timer     = undefined,
-                  txn_executor        = TxnFun,
-                  call                = Call,
-                  cast                = Cast}, hibernate,
+                  txn_executor        = TxnFun }, hibernate,
      {backoff, ?HIBERNATE_AFTER_MIN, ?HIBERNATE_AFTER_MIN, ?DESIRED_HIBERNATE}}.
 
 
@@ -618,14 +606,12 @@ handle_call({add_on_right, NewMember}, _From,
             State = #state { self          = Self,
                              group_name    = GroupName,
                              members_state = MembersState,
-                             txn_executor  = TxnFun,
-                             cast          = Cast}) ->
+                             txn_executor  = TxnFun }) ->
     Group = record_new_member_in_group(NewMember, Self, GroupName, TxnFun),
     View1 = group_to_view(Group),
     MembersState1 = remove_erased_members(MembersState, View1),
     ok = send_right(NewMember, View1,
-                    {catchup, Self, prepare_members_state(MembersState1)},
-                    Cast),
+                    {catchup, Self, prepare_members_state(MembersState1)}),
     {Result, State1} = change_view(View1, State #state {
                                             members_state = MembersState1 }),
     handle_callback_result({Result, {ok, Group}, State1}).
@@ -667,9 +653,8 @@ handle_cast(join, State = #state { self          = Self,
                                    members_state = undefined,
                                    module        = Module,
                                    callback_args = Args,
-                                   txn_executor  = TxnFun,
-                                   call          = Call}) ->
-    View = join_group(Self, GroupName, TxnFun, Call),
+                                   txn_executor  = TxnFun }) ->
+    View = join_group(Self, GroupName, TxnFun),
     MembersState =
         case alive_view_members(View) of
             [Self] -> blank_member_state();
@@ -776,9 +761,8 @@ handle_msg({catchup, Left, MembersStateLeft},
                             left          = {Left, _MRefL},
                             right         = {Right, _MRefR},
                             view          = View,
-                            members_state = undefined,
-                            cast          = Cast}) ->
-    ok = send_right(Right, View, {catchup, Self, MembersStateLeft}, Cast),
+                            members_state = undefined }) ->
+    ok = send_right(Right, View, {catchup, Self, MembersStateLeft}),
     MembersStateLeft1 = build_members_state(MembersStateLeft),
     {ok, State #state { members_state = MembersStateLeft1 }};
 
@@ -1049,17 +1033,15 @@ ensure_alive_suffix1(MembersQ) ->
 %% View modification
 %% ---------------------------------------------------------------------------
 
-join_group(Self, GroupName, TxnFun, Call) ->
-    join_group(Self, GroupName, dirty_read_group(GroupName), TxnFun, Call).
+join_group(Self, GroupName, TxnFun) ->
+    join_group(Self, GroupName, dirty_read_group(GroupName), TxnFun).
 
-join_group(Self, GroupName, {error, not_found}, TxnFun, Call) ->
+join_group(Self, GroupName, {error, not_found}, TxnFun) ->
     join_group(Self, GroupName,
-               prune_or_create_group(Self, GroupName, TxnFun), TxnFun, Call);
-join_group(Self, _GroupName, #gm_group { members = [Self] } = Group,
-           _TxnFun, _Call) ->
+               prune_or_create_group(Self, GroupName, TxnFun), TxnFun);
+join_group(Self, _GroupName, #gm_group { members = [Self] } = Group, _TxnFun) ->
     group_to_view(Group);
-join_group(Self, GroupName, #gm_group { members = Members } = Group,
-           TxnFun, Call) ->
+join_group(Self, GroupName, #gm_group { members = Members } = Group, TxnFun) ->
     case lists:member(Self, Members) of
         true ->
             group_to_view(Group);
@@ -1068,7 +1050,7 @@ join_group(Self, GroupName, #gm_group { members = Members } = Group,
                 [] ->
                     join_group(Self, GroupName,
                                prune_or_create_group(Self, GroupName, TxnFun),
-                               TxnFun, Call);
+                               TxnFun);
                 Alive ->
                     Left = lists:nth(random:uniform(length(Alive)), Alive),
                     Handler =
@@ -1077,14 +1059,13 @@ join_group(Self, GroupName, #gm_group { members = Members } = Group,
                                   Self, GroupName,
                                   record_dead_member_in_group(
                                     Left, GroupName, TxnFun),
-                                  TxnFun, Call)
+                                  TxnFun)
                         end,
                     try
-                        case Call(
+                        case ?INSTR_MOD:call(
                                get_pid(Left), {add_on_right, Self}, infinity) of
                             {ok, Group1} -> group_to_view(Group1);
-                            not_ready    -> join_group(
-                                              Self, GroupName, TxnFun, Call)
+                            not_ready    -> join_group(Self, GroupName, TxnFun)
                         end
                     catch
                         exit:{R, _}
@@ -1202,20 +1183,20 @@ can_erase_view_member(_Self, _Id, _LA, _LP) -> false.
 %% View monitoring and maintanence
 %% ---------------------------------------------------------------------------
 
-ensure_neighbour(_Ver, Self, {Self, undefined}, Self, _Cast) ->
+ensure_neighbour(_Ver, Self, {Self, undefined}, Self) ->
     {Self, undefined};
-ensure_neighbour(Ver, Self, {Self, undefined}, RealNeighbour, Cast) ->
-    ok = Cast(get_pid(RealNeighbour), {?TAG, Ver, check_neighbours}),
+ensure_neighbour(Ver, Self, {Self, undefined}, RealNeighbour) ->
+    ok = ?INSTR_MOD:cast(get_pid(RealNeighbour), {?TAG, Ver, check_neighbours}),
     {RealNeighbour, maybe_monitor(RealNeighbour, Self)};
-ensure_neighbour(_Ver, _Self, {RealNeighbour, MRef}, RealNeighbour, _Cast) ->
+ensure_neighbour(_Ver, _Self, {RealNeighbour, MRef}, RealNeighbour) ->
     {RealNeighbour, MRef};
-ensure_neighbour(Ver, Self, {RealNeighbour, MRef}, Neighbour, Cast) ->
+ensure_neighbour(Ver, Self, {RealNeighbour, MRef}, Neighbour) ->
     true = erlang:demonitor(MRef),
     Msg = {?TAG, Ver, check_neighbours},
-    ok = Cast(get_pid(RealNeighbour), Msg),
+    ok = ?INSTR_MOD:cast(get_pid(RealNeighbour), Msg),
     ok = case Neighbour of
              Self -> ok;
-             _    -> Cast(get_pid(Neighbour), Msg)
+             _    -> ?INSTR_MOD:cast(get_pid(Neighbour), Msg)
          end,
     {Neighbour, maybe_monitor(Neighbour, Self)}.
 
@@ -1226,13 +1207,12 @@ check_neighbours(State = #state { self             = Self,
                                   left             = Left,
                                   right            = Right,
                                   view             = View,
-                                  broadcast_buffer = Buffer,
-                                  cast             = Cast}) ->
+                                  broadcast_buffer = Buffer }) ->
     #view_member { left = VLeft, right = VRight }
         = fetch_view_member(Self, View),
     Ver = view_version(View),
-    Left1 = ensure_neighbour(Ver, Self, Left, VLeft, Cast),
-    Right1 = ensure_neighbour(Ver, Self, Right, VRight, Cast),
+    Left1 = ensure_neighbour(Ver, Self, Left, VLeft),
+    Right1 = ensure_neighbour(Ver, Self, Right, VRight),
     Buffer1 = case Right1 of
                   {Self, undefined} -> [];
                   _                 -> Buffer
@@ -1252,10 +1232,9 @@ maybe_send_catchup(_Right, #state { members_state = undefined }) ->
 maybe_send_catchup(_Right, #state { self          = Self,
                                     right         = {Right, _MRef},
                                     view          = View,
-                                    members_state = MembersState,
-                                    cast          = Cast}) ->
+                                    members_state = MembersState }) ->
     send_right(Right, View,
-               {catchup, Self, prepare_members_state(MembersState)}, Cast).
+               {catchup, Self, prepare_members_state(MembersState)}).
 
 
 %% ---------------------------------------------------------------------------
@@ -1358,12 +1337,11 @@ maybe_send_activity([], _State) ->
     ok;
 maybe_send_activity(Activity, #state { self  = Self,
                                        right = {Right, _MRefR},
-                                       view  = View,
-                                       cast  = Cast}) ->
-    send_right(Right, View, {activity, Self, Activity}, Cast).
+                                       view  = View }) ->
+    send_right(Right, View, {activity, Self, Activity}).
 
-send_right(Right, View, Msg, Cast) ->
-    ok = Cast(get_pid(Right), {?TAG, view_version(View), Msg}).
+send_right(Right, View, Msg) ->
+    ok = ?INSTR_MOD:cast(get_pid(Right), {?TAG, view_version(View), Msg}).
 
 callback(Args, Module, Activity) ->
     Result =
diff --git a/src/gm_qc.erl b/src/gm_qc.erl
index 6a09c286..79e18f12 100644
--- a/src/gm_qc.erl
+++ b/src/gm_qc.erl
@@ -35,9 +35,17 @@
 %% Helpers
 -export([do_join/0, do_leave/1, do_send/2, do_proceed/2]).
 
+%% For insertion into gm
+-export([call/3, cast/2]).
+
 -record(state, {seq, instrumented, outstanding}).
 
 prop_gm_test() ->
+    case ?INSTR_MOD of
+        ?MODULE -> ok;
+        _       -> exit(compile_with_INSTRUMENT_FOR_GC)
+    end,
+    erlang:register(?MODULE, self()),
     ?FORALL(Cmds, commands(?MODULE), gm_test(Cmds)).
 
 gm_test(Cmds) ->
@@ -175,10 +183,8 @@ terminate(_Pid, _Reason)           -> ok.
 %% ---------------------------------------------------------------------------
 
 do_join() ->
-    {Call, Cast} = instrumented_funs(),
     {ok, GM} = gm:start_link(?GROUP, ?MODULE, self(),
-                             fun rabbit_misc:execute_mnesia_transaction/1,
-                             Call, Cast),
+                             fun rabbit_misc:execute_mnesia_transaction/1),
     %% TODO do we need to test the joined callback? What is the joined
     %% callback actually for?
     %% receive
@@ -287,20 +293,17 @@ outstanding_msgs(#state{outstanding = Outstanding}) ->
                   (_GM, {_Tree, _Set}, true)  -> true
               end, false, Outstanding).
 
-instrumented_funs() ->
-    Test = self(),
-    {fun (Pid, Msg, infinity) ->
-             Ref = make_ref(),
-             Test ! {instrumented, self(), Pid, {call, Ref}},
-             receive
-                 {proceed, Ref} -> ok
-             end,
-             gen_server2:call(Pid, Msg, infinity)
-     end,
-     fun (Pid, Msg) ->
-             Test ! {instrumented, self(), Pid, {cast, Msg}},
-             ok
-     end}.
+call(Pid, Msg, infinity) ->
+    Ref = make_ref(),
+    whereis(?MODULE) ! {instrumented, self(), Pid, {call, Ref}},
+    receive
+        {proceed, Ref} -> ok
+    end,
+    gen_server2:call(Pid, Msg, infinity).
+
+cast(Pid, Msg) ->
+    whereis(?MODULE) ! {instrumented, self(), Pid, {cast, Msg}},
+    ok.
 
 timestamp() -> timer:now_diff(os:timestamp(), {0, 0, 0}).