commit 39d8aa0a1ac1c698f5e5e7232ff9cc031133c20d
parent ba7e17160df2088251217d059ad3441253b74993
Author: zeertzjq <zeertzjq@outlook.com>
Date: Sun, 11 Jan 2026 17:40:32 +0800
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.
Diffstat:
2 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c
@@ -161,7 +161,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;
@@ -530,9 +532,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
@@ -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()