From 9a793526c9362d5cbb8c0397bb60e8626a8ead81 Mon Sep 17 00:00:00 2001 From: Kamil Kowalski Date: Thu, 27 Aug 2026 10:37:26 +0200 Subject: [PATCH] fix(gun): scope named connection processes to the local node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GRPC.Client.Adapters.Gun.ConnectionProcess` registered itself under `{:global, {ConnectionProcess, {ref, host, port}}}`. `:global` is cluster-wide, so on a connected cluster the second node to call `GRPC.Stub.connect(addr, name: Foo)` got `{:error, {:already_started, remote_pid}}` and adopted the *other* node's connection process as its `conn_pid`. `connect/2` reported success, but the channel was unusable: `GRPC.Stub.call/5` checks `Process.alive?(conn_pid)` on every RPC, and `Process.alive?/1` raises `ArgumentError` on a remote pid, so every call failed before a request was sent. `GRPC.Client.Connection.channel_alive?/1` has the same problem on DNS re-resolve, but the hot-path check fires first. Registering globally also meant every connect paid a cluster-wide `:global.register_name/3` — a global lock plus a multi-call to every connected node — even for the unique `make_ref/0` names that `connect/2` generates when no `:name` is given, which nothing else ever looks up. `GRPC.Client.Connection` is already registered in the node-local `GRPC.Client.Registry`; register the Gun connection process there too so reuse is per node, matching the intent of the original change (keeping named channels alive after the calling process exits). Co-Authored-By: Claude Opus 5 (1M context) --- grpc/CHANGELOG.md | 5 +++ .../client/adapters/gun/connection_process.ex | 5 ++- grpc/test/grpc/adapters/gun_test.exs | 41 +++++++++++++++++++ grpc/test/grpc/client/connection_test.exs | 21 +++++++++- 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/grpc/CHANGELOG.md b/grpc/CHANGELOG.md index ca5eac2b0..0c53d9a0d 100644 --- a/grpc/CHANGELOG.md +++ b/grpc/CHANGELOG.md @@ -5,6 +5,11 @@ ### Behavior Changes * The Mint adapter now enforces the requested `:timeout`/`:deadline` on unary receives. A unary call that never receives a response fails with `DEADLINE_EXCEEDED` after the documented 10s default instead of blocking indefinitely, and an explicit `:deadline` now takes precedence over `:timeout`. + +### Bug Fixes + + * The Gun adapter no longer shares a named channel's connection process across Erlang nodes. It was registered in `:global`, so a node connecting with a `:name` already used on another node adopted the remote connection process; `connect/2` returned `{:ok, channel}`, but every RPC on it then raised `ArgumentError` because `GRPC.Stub.call/5` calls `Process.alive?/1` on the connection pid and that raises for remote pids. Connection processes are now registered in the node-local `GRPC.Client.Registry`, so reuse is per node. + ## v1.0.4 (2026-0-15) ### Bug Fixes diff --git a/grpc/lib/grpc/client/adapters/gun/connection_process.ex b/grpc/lib/grpc/client/adapters/gun/connection_process.ex index 17dc7308e..e3de391a1 100644 --- a/grpc/lib/grpc/client/adapters/gun/connection_process.ex +++ b/grpc/lib/grpc/client/adapters/gun/connection_process.ex @@ -13,6 +13,9 @@ defmodule GRPC.Client.Adapters.Gun.ConnectionProcess do Request-specific Gun messages are routed to per-stream response processes, so this process only needs to manage connection-level lifecycle and stream bookkeeping. + + Named channels are registered in the node-local `GRPC.Client.Registry` under + `{ref, host, port}`, so a connection is reused by every caller on the node. """ use GenServer @@ -177,7 +180,7 @@ defmodule GRPC.Client.Adapters.Gun.ConnectionProcess do end defp via(channel) do - {:global, {__MODULE__, owner_key(channel)}} + {:via, Registry, {GRPC.Client.Registry, {__MODULE__, owner_key(channel)}}} end defp start_response_process do diff --git a/grpc/test/grpc/adapters/gun_test.exs b/grpc/test/grpc/adapters/gun_test.exs index 48867e800..8aca4912e 100644 --- a/grpc/test/grpc/adapters/gun_test.exs +++ b/grpc/test/grpc/adapters/gun_test.exs @@ -2,6 +2,7 @@ defmodule GRPC.Client.Adapters.GunTest do use GRPC.Client.DataCase, async: true alias GRPC.Client.Adapters.Gun + alias GRPC.Client.Adapters.Gun.ConnectionProcess defmodule Endpoint do use GRPC.Endpoint @@ -97,6 +98,46 @@ defmodule GRPC.Client.Adapters.GunTest do ] ) end + + test "reuses the connection process for a named channel", %{ + port: port, + credential: credential + } do + channel = + build(:channel, port: port, host: "localhost", cred: credential, ref: make_ref()) + + assert {:ok, %{adapter_payload: %{conn_pid: conn_pid}} = connected} = + Gun.connect(channel, []) + + on_exit(fn -> Gun.disconnect(connected) end) + + assert {:ok, %{adapter_payload: %{conn_pid: ^conn_pid}}} = Gun.connect(channel, []) + + assert [{^conn_pid, _}] = + Registry.lookup( + GRPC.Client.Registry, + {ConnectionProcess, {channel.ref, channel.host, channel.port}} + ) + end + + test "does not reuse the connection process for an unnamed channel", %{ + port: port, + credential: credential + } do + channel = build(:channel, port: port, host: "localhost", cred: credential) + + assert {:ok, %{adapter_payload: %{conn_pid: first_pid}} = first} = Gun.connect(channel, []) + + assert {:ok, %{adapter_payload: %{conn_pid: second_pid}} = second} = + Gun.connect(channel, []) + + on_exit(fn -> + Gun.disconnect(first) + Gun.disconnect(second) + end) + + refute first_pid == second_pid + end end describe "disconnect/1" do diff --git a/grpc/test/grpc/client/connection_test.exs b/grpc/test/grpc/client/connection_test.exs index d9b0fe0b6..3067e7c4d 100644 --- a/grpc/test/grpc/client/connection_test.exs +++ b/grpc/test/grpc/client/connection_test.exs @@ -378,11 +378,28 @@ defmodule GRPC.Client.ConnectionTest do target = "ipv4:127.0.0.1:#{port}" ref = :shared_channel - assert {:ok, %Channel{ref: ^ref}} = + assert {:ok, %Channel{ref: ^ref} = channel1} = @peer.call(peer1, Connection, :connect, [target, [name: ref]]) - assert {:ok, %Channel{ref: ^ref}} = + assert {:ok, %Channel{ref: ^ref} = channel2} = @peer.call(peer2, Connection, :connect, [target, [name: ref]]) + + # Each node must own its own adapter connection. Adopting another node's + # connection process leaves the channel unusable: `GRPC.Stub.call/5` + # checks `Process.alive?(conn_pid)` on every RPC, and `Process.alive?/1` + # raises ArgumentError on a remote pid. + %{adapter_payload: %{conn_pid: conn_pid1}} = channel1 + %{adapter_payload: %{conn_pid: conn_pid2}} = channel2 + + assert node(conn_pid1) == node1 + assert node(conn_pid2) == node2 + + point = %Routeguide.Point{latitude: 409_146_138, longitude: -746_188_906} + + for {peer, channel} <- [{peer1, channel1}, {peer2, channel2}] do + assert {:ok, %Routeguide.Feature{}} = + @peer.call(peer, Routeguide.RouteGuide.Stub, :get_feature, [channel, point]) + end end end