rpc: do not abort on shutdown when a teardown round trip fails

[danielhanchen]

What happens

llama-server built with GGML_RPC aborts on exit, after it has already printed
cleaning up before exit:

ggml/src/ggml-rpc/ggml-rpc.cpp:891: Remote RPC server crashed or returned malformed response
#3  ggml_backend_rpc_get_device_memory ()
#4  common_memory_breakdown_print(llama_context const*) ()
#5  llama_server(common_params&, int, char**) ()

Why

common_memory_breakdown_print() is called after clean_up(), which calls
llama_backend_free(). The RPC device answers get_memory with a round trip to the peer,
and by that point the round trip fails, so RPC_STATUS_ASSERT aborts the process.

This was first seen on a two node layer split where the peer ggml-rpc-server had been
stopped before the local server exited, so it looked like a consequence of the peer going
away. It is not. It reproduces with a peer that is alive and healthy for the whole run,
which makes it a deterministic abort on every shutdown of a server that uses --rpc.

The fix

Device memory is an informational property, not part of the data path, and
ggml_backend_rpc_get_device_memory() already has a defined answer for an endpoint it
cannot reach at all: report 0 free and 0 total. Report the same when the query itself
fails, and log an error.

Freeing a remote buffer is released state and is also a teardown operation, so a failure
there is logged instead of aborting: if the peer is gone then so is the buffer, and the
local context is freed either way. Without this second hunk the process still aborts a few
microseconds later in ggml_backend_rpc_buffer_free_buffer from
ggml_backend_sched_free, on the way out of ~llama_context.

Every other RPC_STATUS_ASSERT, including all of the data path ones, is unchanged. A
failure while setting or getting a tensor or running a graph still aborts.

The alternative, teaching common_memory_breakdown_print() to skip backends whose
connection has dropped, was rejected: that code is shared with every non-RPC build and has
no way to ask a backend whether it is still reachable, so it would need a new backend
interface entry for one caller. Caching the last known figure inside the RPC backend was
also rejected: it adds state to keep in sync in order to print a number that is meaningless
by the time it is printed.

What was measured

CPU only, stories15M-q4_0.gguf, a local ggml-rpc-server on 127.0.0.1, server started
with --rpc 127.0.0.1:PORT --device RPC0 -ngl 99, one completion served, then SIGINT.

• Before, peer killed first: abort, core dumped, stack as above.
• Before, peer alive throughout: abort, core dumped, same stack.
• After, peer killed first: clean exit, one failed to query device memory of ..., reporting 0
  and three failed to free the remote buffer, the connection is gone.
• After, peer alive throughout: clean exit.

Non-RPC builds

The change touches one file, ggml/src/ggml-rpc/ggml-rpc.cpp. That file is compiled only
through ggml_add_backend(RPC) in ggml/src/CMakeLists.txt, which adds the subdirectory
only if GGML_RPC is set, and GGML_RPC defaults to OFF in ggml/CMakeLists.txt. No
header, no public signature and no shared code path changes, so NVIDIA, AMD and CPU only
builds without RPC are byte for byte unaffected.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for security reviews. Please try again later.