pg leaves phantom empty groups visible in which_groups/1 after a remote node's last member in that group is removed

[ceolinrenato]

Describe the bug

After a node crashes/leaves the cluster, other nodes continue to list that Node's group in pg:which_groups/1 even though it has no members anymore. Only pg:get_members/2 correctly reveals the group is empty.

To Reproduce

Sorry I don't have much exp with ERL so I reproduced this using Elixir's iex shell

    # 4 terminals:
    iex --sname n1@localhost
    iex --sname n2@localhost
    iex --sname n3@localhost
    iex --sname n4@localhost

    # on n1:
    Node.connect(:n2@localhost); Node.connect(:n3@localhost); Node.connect(:n4@localhost)
    :pg.start_link(:repro_scope)
    :pg.join(:repro_scope, {:member, Node.self()}, self())
    # (repeat the :pg.start_link/:pg.join on n2, n3, n4, each joining {:member, Node.self()})

    # on n1, confirm baseline:
    :pg.which_groups(:repro_scope)
    #=> [member: :n3@localhost, member: :n1@localhost, member: :n2@localhost, member: :n4@localhost]

    # kill n2's BEAM process directly (e.g. `kill -9 <pid>`, or SIGKILL via
    # htop) -- do NOT use System.halt()/graceful shutdown, though this is
    # expected to behave identically since pg:terminate/2 does not
    # broadcast a leave on shutdown either way

    # back on n1, after the {'DOWN', peer} monitor for n2 has fired
    # (confirm via `:erlang.process_info(Process.whereis(:repro_scope), :monitors)`
    # no longer listing n2's peer pid):
    :pg.which_groups(:repro_scope)
    #=> STILL includes `member: :n2@localhost`

    :pg.get_members(:repro_scope, {:member, :n2@localhost})
    #=> [] -- confirms the row exists but is empty (phantom group)

Expected behavior

which_groups/1 should not list groups whose member list is empty

Affected versions

This was reproduced on 29.0.2

Additional context

Ran this through an LLM model and it found out this, but I'm not sure if relevant:

pg:update_global_view_and_notify/4 has two clauses: one for updates
originating on the local node (Node =:= node()), and one for updates
originating on a remote node. The local-update clause correctly deletes
the ETS row for a group when its member list becomes empty:

case Add ++ (All -- Remove) of
    [] -> ets:delete(Scope, Group);
    NewAll -> ets:insert(Scope, {Group, NewAll, Add ++ (Local -- Remove)})
end

but the remote-update clause does not have this check, and unconditionally
inserts, even when empty:

case ets:lookup(Scope, Group) of
    [{_Group, All, Local}] ->
        ets:insert(Scope, {Group, Add ++ (All -- Remove), Local});
    _ ->
        ets:insert(Scope, {Group, Add, []})
end

As a result, once a remote node's last member of a group leaves (e.g. the
node disconnects and its peer monitor fires, or the process explicitly
leaves), the ETS row for that group is updated to {Group, [], Local}
instead of being deleted. pg:which_groups/1 matches on
ets:match(Scope, {'$1', '_', '_'}) with no filtering on non-empty member
lists, so the now-empty group remains permanently visible in
which_groups/1, even though get_members/2 on the same group correctly
returns [].

This means any code relying on pg:which_groups/1 to enumerate "currently
active" groups (e.g. cluster membership tracking, as in our case: mapping
{member, Node} groups to determine live cluster nodes) will accumulate
permanently stale phantom entries for every node/process that has ever
disconnected, with no way to distinguish a phantom entry from a live one
without an extra get_members/2 call per group.

[ceolinrenato]

Update I was able to modify an existing test on lib/kernel/test/pg_SUITE.erl and reproduce the issue

Felt inclined to open a PR with a fix after running the suite again and realize there was no regressions

[ceolinrenato]

erlang's reproduction steps

%% 4 terminals:
erl -sname n1@localhost
erl -sname n2@localhost
erl -sname n3@localhost
erl -sname n4@localhost

%% on n1:
net_kernel:connect_node('n2@localhost'),
net_kernel:connect_node('n3@localhost'),
net_kernel:connect_node('n4@localhost').

pg:start_link(repro_scope).
pg:join(repro_scope, {member, node()}, self()).
%% (repeat pg:start_link/1 and pg:join/3 on n2, n3, n4, each joining {member, node()})

%% on n1, confirm baseline:
pg:which_groups(repro_scope).
%%=> [{member,'n3@localhost'},{member,'n1@localhost'},
%%    {member,'n2@localhost'},{member,'n4@localhost'}]

%% kill or stop n2's BEAM process 

%% back on n1, after the 'DOWN' monitor for n2 has fired
%% (confirm via):
erlang:process_info(whereis(repro_scope), monitors).
%% should no longer list n2's peer pid

pg:which_groups(repro_scope).
%%=> STILL includes {member,'n2@localhost'}

pg:get_members(repro_scope, {member, 'n2@localhost'}).
%%=> [] -- confirms the row exists but is empty (phantom group)