rpc: Don't delay notifications when request is pending (#6544)

With the old behavior, if a GUI makes a blocking request that requires user
interaction (like nvim_input()), it would not get any screen updates.

The client, not nvim, should decide how to handle notifications during a
pending request. If an rplugin wants to avoid async calls while a sync call is
busy, it likely wants to avoid processing async calls while another async call
also is handled as well.

This may break the expectation of some existing rplugins. For compatibility,
remote/define.vim reimplements the old behavior. Clients can opt-out by
specifying `sync=urgent`.

- Legacy hosts should be updated to use `sync=urgent`. They could add a flag
  indicating which async methods are always safe to call and which must wait
  until the main loop returns.
- New hosts can expose the full asyncness, they don't need to offer both
  behaviors.

ref #6532
ref #1398 d83868fe9071af1b4866594eac12f7aa0fa71b53

1 files changed, 42 insertions, 19 deletions

diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua
index 9f245d913b..4380e52b8b 100644
--- a/test/functional/api/server_requests_spec.lua
+++ b/test/functional/api/server_requests_spec.lua
@@ -109,7 +109,28 @@ describe('server -> client', function()
   end)
 
   describe('requests and notifications interleaved', function()
-    -- This tests that the following scenario won't happen:
+    it('does not delay notifications during pending request', function()
+      local received = false
+      local function on_setup()
+        eq("retval", funcs.rpcrequest(cid, "doit"))
+        stop()
+      end
+      local function on_request(method)
+        if method == "doit" then
+          funcs.rpcnotify(cid, "headsup")
+          eq(true,received)
+          return "retval"
+        end
+      end
+      local function on_notification(method)
+        if method == "headsup" then
+          received = true
+        end
+      end
+      run(on_request, on_notification, on_setup)
+    end)
+
+    -- This tests the following scenario:
     --
     -- server->client [request     ] (1)
     -- client->server [request     ] (2) triggered by (1)
@@ -124,36 +145,38 @@ describe('server -> client', function()
     -- only deals with one server->client request at a time. (In other words,
     -- the client cannot send a response to a request that is not at the top
     -- of nvim's request stack).
-    --
-    -- But above scenario shoudn't happen by the way notifications are dealt in
-    -- Nvim: they are only sent after there are no pending server->client
-    -- request(the request stack fully unwinds). So (3) is only sent after the
-    -- client returns (6).
-    it('works', function()
-      local expected = 300
-      local notified = 0
+    pending('will close connection if not properly synchronized', function()
       local function on_setup()
         eq('notified!', eval('rpcrequest('..cid..', "notify")'))
       end
 
       local function on_request(method)
-        eq('notify', method)
-        eq(1, eval('rpcnotify('..cid..', "notification")'))
-        return 'notified!'
+        if method == "notify" then
+          eq(1, eval('rpcnotify('..cid..', "notification")'))
+          return 'notified!'
+        elseif method == "nested" then
+          -- do some busywork, so the first request will return
+          -- before this one
+          for _ = 1, 5 do
+            eq(2, eval("1+1"))
+          end
+          eq(1, eval('rpcnotify('..cid..', "nested_done")'))
+          return 'done!'
+        end
       end
 
       local function on_notification(method)
-        eq('notification', method)
-        if notified == expected then
-          stop()
-          return
+        if method == "notification" then
+          eq('done!', eval('rpcrequest('..cid..', "nested")'))
+        elseif method == "nested_done" then
+          -- this should never have been sent
+          ok(false)
         end
-        notified = notified + 1
-        eq('notified!', eval('rpcrequest('..cid..', "notify")'))
       end
 
       run(on_request, on_notification, on_setup)
-      eq(expected, notified)
+      -- ignore disconnect failure, otherwise detected by after_each
+      clear()
     end)
   end)