From 2a3cd8dc80fbcc224f46d049c895e189f4b87d8b Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Sun, 11 Jan 2026 17:40:32 +0800 Subject: [PATCH] fix(rpc): don't overwrite already received results on error (#37339) This fixes a regression from cf6f60ce4dc376570e8d71facea76299ca951604 (possibly), as before that commit a frame is popped from the call stack immediately after its response is received. Also fix leaking the allocated error messages. (cherry picked from commit 39d8aa0a1ac1c698f5e5e7232ff9cc031133c20d) --- src/nvim/msgpack_rpc/channel.c | 10 +++- test/functional/api/server_requests_spec.lua | 48 ++++++++++++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c index 3efd5f5628..5f3be02048 100644 --- a/src/nvim/msgpack_rpc/channel.c +++ b/src/nvim/msgpack_rpc/channel.c @@ -163,7 +163,9 @@ Object rpc_send_call(uint64_t id, const char *method_name, Array args, ArenaMem LOOP_PROCESS_EVENTS_UNTIL(&main_loop, channel->events, -1, frame.returned || rpc->closed); (void)kv_pop(rpc->call_stack); - if (rpc->closed) { + // !frame.returned implies rpc->closed. + // If frame.returned is true, its memory needs to be freed, so don't return here. + if (!frame.returned) { api_set_error(err, kErrorTypeException, "Invalid channel: %" PRIu64, id); channel_decref(channel); return NIL; @@ -526,9 +528,13 @@ static void chan_close_on_err(Channel *channel, char *msg, int loglevel) { for (size_t i = 0; i < kv_size(channel->rpc.call_stack); i++) { ChannelCallFrame *frame = kv_A(channel->rpc.call_stack, i); + if (frame->returned) { + continue; // Don't overwrite an already received result. + } frame->returned = true; frame->errored = true; - frame->result = CSTR_TO_OBJ(msg); + frame->result = CSTR_TO_ARENA_OBJ(&channel->rpc.unpacker->arena, msg); + frame->result_mem = arena_finish(&channel->rpc.unpacker->arena); } channel_close(channel->id, kChannelPartRpc, NULL); diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index 3e81cd081c..95435509d6 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -362,7 +362,7 @@ describe('server -> client', function() connect_test(server, 'tcp', address) end) - it('does not crash on receiving UI events', function() + local function start_server_and_client() local server = n.new_session(false) set_session(server) local address = fn.serverlist()[1] @@ -370,11 +370,53 @@ describe('server -> client', function() set_session(client) local id = fn.sockconnect('pipe', address, { rpc = true }) + + finally(function() + server:close() + client:close() + end) + + return id + end + + it('does not crash on receiving UI events', function() + local id = start_server_and_client() fn.rpcrequest(id, 'nvim_ui_attach', 80, 24, {}) assert_alive() + end) - server:close() - client:close() + it('does not leak memory with channel closed before response', function() + local id = start_server_and_client() + eq( + ('ch %d was closed by the peer'):format(id), + pcall_err(n.exec_lua, function() + vim.rpcrequest(id, 'nvim_command', 'qall!') + end) + ) + eq({}, api.nvim_get_chan_info(id)) -- Channel is closed. + end) + + it('response works with channel closed just after response', function() + local id = start_server_and_client() + eq( + 'RESPONSE', + n.exec_lua(function() + local prepare = vim.uv.new_prepare() + -- Block the event loop after writing the request but before polling for I/O + -- so that response and EOF arrive at the same uv_run() call. + prepare:start(function() + vim.uv.sleep(50) + prepare:close() + end) + return vim.rpcrequest( + id, + 'nvim_exec_lua', + [[vim.schedule(function() vim.cmd('qall!') end); return 'RESPONSE']], + {} + ) + end) + ) + eq({}, api.nvim_get_chan_info(id)) -- Channel is closed. end) it('via stdio, with many small flushes does not crash #23781', function()