fix(rpc): don't overwrite already received results on error (#37339)

This fixes a regression from cf6f60ce4d
(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 39d8aa0a1a)

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);

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()
+    it('does not leak memory with channel closed before response', function()
-      client:close()
+      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()