Browse Source

Fix leaving multiple groups when processes die.

[closes ostinelli/syn#14]
Roberto Ostinelli 9 years ago
parent
commit
bd61a1d160
3 changed files with 52 additions and 41 deletions
  1. 1 1
      README.md
  2. 37 33
      src/syn_groups.erl
  3. 14 7
      test/syn_groups_SUITE.erl

+ 1 - 1
README.md

@@ -239,7 +239,7 @@ syn:leave(Name, Pid) -> ok | {error, Error}.
 Types:
 	Name = any()
 	Pid = pid()
-	Error = undefined | pid_not_in_group
+	Error = pid_not_in_group
 ```
 
 > You don't need to remove processes that are about to die, since they are monitored by Syn and they will be removed automatically from their groups.

+ 37 - 33
src/syn_groups.erl

@@ -154,11 +154,8 @@ handle_call({join, Name, Pid}, _From, State) ->
     end;
 
 handle_call({leave, Name, Pid}, _From, State) ->
-    %% we check again to return the correct response regardless of race conditions
-    case i_find_by_pid(Pid) of
+    case find_by_pid_and_name(Pid, Name) of
         undefined ->
-            {reply, {error, undefined}, State};
-        Process when Process#syn_groups_table.name =/= Name ->
             {error, pid_not_in_group};
         Process ->
             %% remove from table
@@ -195,8 +192,8 @@ handle_cast(Msg, State) ->
 
 handle_info({'EXIT', Pid, Reason}, State) ->
     %% check if pid is in table
-    case i_find_by_pid(Pid) of
-        undefined ->
+    case find_groups_by_pid(Pid) of
+        [] ->
             %% log
             case Reason of
                 normal -> ok;
@@ -205,18 +202,21 @@ handle_info({'EXIT', Pid, Reason}, State) ->
                     error_logger:error_msg("Received an exit message from an unlinked process ~p with reason: ~p", [Pid, Reason])
             end;
 
-        Process ->
-            %% get group
-            Name = Process#syn_groups_table.name,
-            %% log
-            case Reason of
-                normal -> ok;
-                killed -> ok;
-                _ ->
-                    error_logger:error_msg("Process of group ~p and pid ~p exited with reason: ~p", [Name, Pid, Reason])
+        Processes ->
+            F = fun(Process) ->
+                %% get group
+                Name = Process#syn_groups_table.name,
+                %% log
+                case Reason of
+                    normal -> ok;
+                    killed -> ok;
+                    _ ->
+                        error_logger:error_msg("Process of group ~p and pid ~p exited with reason: ~p", [Name, Pid, Reason])
+                end,
+                %% delete from table
+                remove_process(Process)
             end,
-            %% delete from table
-            remove_process(Process)
+            lists:foreach(F, Processes)
     end,
     %% return
     {noreply, State};
@@ -244,21 +244,28 @@ code_change(_OldVsn, State, _Extra) ->
 %% ===================================================================
 %% Internal
 %% ===================================================================
--spec i_member(Pid :: pid(), Name :: any()) -> boolean().
-i_member(Pid, Name) when is_tuple(Name) ->
-    i_member_check(Pid, {'==', '$1', {Name}});
-i_member(Pid, Name) ->
-    i_member_check(Pid, {'=:=', '$1', Name}).
-
--spec i_member_check(Pid :: pid(), NameGuard :: any()) -> boolean().
-i_member_check(Pid, NameGuard) ->
+-spec find_by_pid_and_name(Pid :: pid(), Name :: any()) -> Process :: #syn_groups_table{} | undefined.
+find_by_pid_and_name(Pid, Name) when is_tuple(Name) ->
+    i_find_by_pid_and_name(Pid, {'==', '$1', {Name}});
+find_by_pid_and_name(Pid, Name) ->
+    i_find_by_pid_and_name(Pid, {'=:=', '$1', Name}).
+
+-spec i_find_by_pid_and_name(Pid :: pid(), NameGuard :: any()) -> boolean().
+i_find_by_pid_and_name(Pid, NameGuard) ->
     %% build match specs
     MatchHead = #syn_groups_table{name = '$1', pid = '$2', _ = '_'},
     Guards = [NameGuard, {'=:=', '$2', Pid}],
-    Result = '$2',
+    Result = '$_',
     %% select
     case mnesia:dirty_select(syn_groups_table, [{MatchHead, Guards, [Result]}]) of
-        [] -> false;
+        [] -> undefined;
+        [Process] -> Process
+    end.
+
+-spec i_member(Pid :: pid(), Name :: any()) -> boolean().
+i_member(Pid, Name) ->
+    case find_by_pid_and_name(Pid, Name) of
+        undefined -> false;
         _ -> true
     end.
 
@@ -270,12 +277,9 @@ i_get_members(Name) ->
     end, Processes),
     lists:sort(Pids).
 
--spec i_find_by_pid(Pid :: pid()) -> Process :: #syn_groups_table{} | undefined.
-i_find_by_pid(Pid) ->
-    case mnesia:dirty_index_read(syn_groups_table, Pid, #syn_groups_table.pid) of
-        [Process] -> Process;
-        _ -> undefined
-    end.
+-spec find_groups_by_pid(Pid :: pid()) -> [Process :: #syn_groups_table{}].
+find_groups_by_pid(Pid) ->
+    mnesia:dirty_index_read(syn_groups_table, Pid, #syn_groups_table.pid).
 
 -spec remove_process(Process :: #syn_groups_table{}) -> ok.
 remove_process(Process) ->

+ 14 - 7
test/syn_groups_SUITE.erl

@@ -208,19 +208,26 @@ single_node_kill(_Config) ->
     %% start process
     Pid = syn_test_suite_helper:start_process(),
     %% retrieve
-    [] = syn:get_members(<<"my group">>),
-    false = syn:member(Pid, <<"my group">>),
+    [] = syn:get_members(<<"my group 1">>),
+    [] = syn:get_members(<<"my group 2">>),
+    false = syn:member(Pid, <<"my group 1">>),
+    false = syn:member(Pid, <<"my group 2">>),
     %% join
-    ok = syn:join(<<"my group">>, Pid),
+    ok = syn:join(<<"my group 1">>, Pid),
+    ok = syn:join(<<"my group 2">>, Pid),
     %% retrieve
-    [Pid] = syn:get_members(<<"my group">>),
-    true = syn:member(Pid, <<"my group">>),
+    [Pid] = syn:get_members(<<"my group 1">>),
+    [Pid] = syn:get_members(<<"my group 2">>),
+    true = syn:member(Pid, <<"my group 1">>),
+    true = syn:member(Pid, <<"my group 2">>),
     %% kill process
     syn_test_suite_helper:kill_process(Pid),
     timer:sleep(100),
     %% retrieve
-    [] = syn:get_members(<<"my group">>),
-    false = syn:member(Pid, <<"my group">>).
+    [] = syn:get_members(<<"my group 1">>),
+    [] = syn:get_members(<<"my group 2">>),
+    false = syn:member(Pid, <<"my group 1">>),
+    false = syn:member(Pid, <<"my group 2">>).
 
 single_node_publish(_Config) ->
     %% set schema location