File 3412-pg-fix-empty-group-removal-from-internal-p... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 3412-pg-fix-empty-group-removal-from-internal-pg-state.patch of Package erlang (Revision fb8f015ac8ccbb72d49ed8284e9f5df5)

Currently displaying revision fb8f015ac8ccbb72d49ed8284e9f5df5 , Show latest

From 6c4464028b4f7d83ef617dd65af762d5ff112ae3 Mon Sep 17 00:00:00 2001
From: Maxim Fedorov <dane@whatsapp.com>
Date: Thu, 11 Mar 2021 13:25:11 -0800
Subject: [PATCH] pg: fix empty group removal from internal pg state

pg keeps two copies of state: gen_server state and ETS table for
concurrent access. #2866 fixed one potential way to introduce
mismatch between internal state and ETS table, and this patch
completes the fix.
---
 lib/kernel/src/pg.erl        | 27 ++++++++++++++-------------
 lib/kernel/test/pg_SUITE.erl | 10 ++++++++++
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/lib/kernel/src/pg.erl b/lib/kernel/src/pg.erl
index 2ca0b083ad..573d2e6953 100644
--- a/lib/kernel/src/pg.erl
+++ b/lib/kernel/src/pg.erl
@@ -281,19 +281,7 @@ handle_info({leave, Peer, PidOrPids, Groups}, #state{scope = Scope, nodes = Node
     case maps:get(Peer, Nodes, []) of
         {MRef, RemoteMap} ->
             _ = leave_remote(Scope, PidOrPids, Groups),
-            NewRemoteMap = lists:foldl(
-                fun (Group, Acc) ->
-                    case maps:get(Group, Acc) of
-                        PidOrPids ->
-                            maps:remove(Group, Acc);
-                        [PidOrPids] ->
-                            maps:remove(Group, Acc);
-                        Existing when is_pid(PidOrPids) ->
-                            Acc#{Group => lists:delete(PidOrPids, Existing)};
-                        Existing ->
-                            Acc#{Group => Existing-- PidOrPids}
-                    end
-                end, RemoteMap, Groups),
+            NewRemoteMap = leave_update_remote_map(PidOrPids, RemoteMap, Groups),
             {noreply, State#state{nodes = Nodes#{Peer => {MRef, NewRemoteMap}}}};
         [] ->
             %% Handle race condition: remote node disconnected, but scope process
@@ -524,6 +512,19 @@ leave_remote(Scope, Pids, Groups) ->
         end ||
         Group <- Groups].
 
+leave_update_remote_map(Pid, RemoteMap, Groups) when is_pid(Pid) ->
+    leave_update_remote_map([Pid], RemoteMap, Groups);
+leave_update_remote_map(Pids, RemoteMap, Groups) ->
+    lists:foldl(
+        fun (Group, Acc) ->
+            case maps:get(Group, Acc) -- Pids of
+                [] ->
+                    maps:remove(Group, Acc);
+                Remaining ->
+                    Acc#{Group => Remaining}
+            end
+        end, RemoteMap, Groups).
+
 all_local_pids(Monitors) ->
     maps:to_list(maps:fold(
         fun(Pid, {_Ref, Groups}, Acc) ->
diff --git a/lib/kernel/test/pg_SUITE.erl b/lib/kernel/test/pg_SUITE.erl
index f03b8a6a39..3a69e33bcf 100644
--- a/lib/kernel/test/pg_SUITE.erl
+++ b/lib/kernel/test/pg_SUITE.erl
@@ -295,6 +295,16 @@ empty_group_by_remote_leave(Config) when is_list(Config) ->
     % empty group should be deleted.
     ?assertEqual(#{}, NewRemoteMap),
 
+    %% another variant of emptying a group remotely: join([Pi1, Pid2]) and leave ([Pid2, Pid1])
+    RemotePid2 = erlang:spawn(TwoPeer, forever()),
+    ?assertEqual(ok, rpc:call(TwoPeer, pg, join, [?FUNCTION_NAME, ?FUNCTION_NAME, [RemotePid, RemotePid2]])),
+    sync({?FUNCTION_NAME, TwoPeer}),
+    ?assertEqual([RemotePid, RemotePid2], pg:get_members(?FUNCTION_NAME, ?FUNCTION_NAME)),
+    %% now leave
+    ?assertEqual(ok, rpc:call(TwoPeer, pg, leave, [?FUNCTION_NAME, ?FUNCTION_NAME, [RemotePid2, RemotePid]])),
+    sync({?FUNCTION_NAME, TwoPeer}),
+    ?assertEqual([], pg:get_members(?FUNCTION_NAME, ?FUNCTION_NAME)),
+    {state, _, _, #{RemoteNode := {_, NewRemoteMap}}} = sys:get_state(?FUNCTION_NAME),
     stop_node(TwoPeer, Socket),
     ok.
 
-- 
2.26.2

Places

File 3412-pg-fix-empty-group-removal-from-internal-pg-state.patch of Package erlang (Revision fb8f015ac8ccbb72d49ed8284e9f5df5)

Places