It's been quite some time since I looked through the code, so I may be missing something and if so, please let me know.

I don't think I understand this PR, why do you need a function marked with FUNC_API_ASYNC to be sync in some cases? If the goal is simply allow embedder to determine the current editor mode (and I assume the reason for that is to know if a certain request would block) why do you need to flush the input buffer?

If the goal is simply allow embedder to determine the current editor mode (and I assume the reason for that is to know if a certain request would block) why do you need to flush the input buffer?

That is the goal, in particular the RPC client needs to know if the next synchronous RPC will block. There could be input in the queue which wasn't flushed yet, which will cause the next synchronous to RPC block. For example:

nvim_input('g')
nvim_get_mode()      // "g" hasn't been processed yet
nvim_command('foo')  // this will block

In the above sequence, if nvim_get_mode() does not flush the input queue, it will incorrectly return blocking=false. Based on that result, the client thinks it can safely call nvim_command()... but in fact it will be blocked after the "g" input is flushed.

For single-threaded clients like actualvim, this is a problem because user input is sent by the client. If the client is blocked then it's stuck.

Another use-case is the neovim test suite: currently the tests have no way to detect a press-enter prompt before it happens.

There could be input in the queue which wasn't flushed yet, which will cause the next synchronous to RPC block.

OK I understand the issue now. Even so, I'm unsure if this is the best approach.

When I started the pushdown automaton refactor(@8d93621c6368fb6e3e3f7138bff50e08585f347b), the goal was to add explicit support for K_EVENT in more states, in the hopes we can eventually remove FUNC_ATTR_ASYNC. Maybe we can try to follow this direction instead?

For now I suggest to investigate the amount of work required to add support for K_EVENT to the various operator pending modes(assuming operator pending is the main source of problems here), and if it is a lot of work, try to create a plan for refactoring. I actually had a lot of ideas on how to remodel the code to make this easier, but I need to refresh memory first before elaborating further.

I didn't forget about the K_EVENT refactoring intentions, but that's a high-risk task. Adding a new API function is much lower risk, and also forward-compatible with whatever future refactoring may occur.

We also need to un-block https://github.com/lunixbochs/actualvim (and who-knows-what else: this has been a known issue for ~2 years).

Any comments on #6247 (comment) ? Could fast_events be leveraged somehow?

I didn't forget about the K_EVENT refactoring intentions, but that's a high-risk task. Adding a new API function is much lower risk, and also forward-compatible with whatever future refactoring may occur.

It may not be as high-risk as you think, I actually had written down a plan on how to incrementally move vim implicit modes into the state_enter loop(the plan is written in the commit message for(@8d93621). In fact, I think it is safer to create an explicit states for some of the operator-pending modes than to process events in API calls marked as async.

Any comments on #6247 (comment) ? Could fast_events be leveraged somehow?

I'm currently doing some reading on the code and on the related issues to refresh my memory and will come back with a response later today.

ActualVim will now detect and use this function if available. It works great for me, and is reliably faster than my heuristic method.

lunixbochs/ActualVim@8a21e68#diff-b00fc497b0226e8afc26fccf90e2c4b0R300

A small update: I'm currently working on a PR that aims to fix #6166/#6159 by doing a ambitious refactoring of normal mode, but still need some days before have something to show. This is something I had planned in 2015 but ended up never finishing, and reading the code on friday/saturday helped me refresh my memory.

It is not the simple refactoring I commented about, but I strongly believe it is the best solution for all problems related to async integration with nvim's main loop(it might even allow removing the distinction between immediate/deferred RPC calls in the short term). It is also something that will greatly help in cleaning up some other very old pieces of code.

Right now it is hard to estimate how many more days I need to finish this, but I will have another update by the end of the week.

@tarruda That is great news.

I pushed an update to this PR that abuses Event.priority so that input_poll() only processes async events. That should be safe, right?

If that is sound I'm thinking we should merge this and can remove the ugly stuff once the pretty stuff is ready.

I pushed an update to this PR that abuses Event.priority so that input_poll() only processes async events. That should be safe, right?

It is difficult to tell with certainty. Apparently it is safe, but I think it is irrelevant.

I suspect solution implemented here doesn't work. If I understood correctly, you are trying to ensure nvim_get_mode is only handled after nvim main loop has fully processed all pending input, but the code you added here doesn't seem to guarantee that. Let's consider the example you gave before:

nvim_input('g')
nvim_get_mode()      // "g" hasn't been processed yet
nvim_command('foo')  // this will block

Imagine the following chain of events:

• Nvim calls os_inchar to wait for input or events
• RPC for nvim_input is received, which is handled immediately by posting data to the input buffer

What happens next depends on OS races that are out of our control. The two possible scenarios are:

• The OS buffered nvim_get_mode before libuv poll returns to process nvim_input. In this case, nvim_get_mode will be processed in the same loop as nvim_input.
• The OS buffered nvim_get_mode after libuv poll returns to process nvim_input. In this case, nvim_get_mode will be processed in the next os_inchar call.

The only way you will get the result you expect(nvim_get_mode returns the correct mode, which is pending 'g' command), is in the second scenario, but calling multiqueue_process_event is irrelevant since the queue will be empty in any of the two situations. Not only the processing of nvim_input happened before nvim_get_mode, but even if it was after, processing nvim_input would simply put data to input buffer, not force the main loop to process it.

I could be missing something, of course(again, it has been a long time :)), but my theory can be verified by making sure that the requests for nvim_input and nvim_get_mode are written to nvim pipe in a single batch(assuming the OS pipe buffer is large enough, which is probably true).

Note: Right now Event.priority is not used, and if I remember correctly it was created it before the parent/child queue concept, which gives a better way to selectively process events. The reason Event.priority still exists is because forgot to remove it, so it should be removed it in a future PR.

The only way you will get the result you expect(nvim_get_mode returns the correct mode, which is pending 'g' command), is in the second scenario, but calling multiqueue_process_event is irrelevant since the queue will be empty in any of the two situations.

It can't be irrelevant, because commenting out that line (multiqueue_process_priority in the latest commit) in input.c causes the tests to hang (at the nvim_get_mode RPC call--i.e. #6159)--which is exactly the thing the tests are verifying. And the queue definitely isn't empty, this can be observed by adding ILOG in multiqueue_process_priority to see the processed event(s).

Regarding scenario 1:

The OS buffered nvim_get_mode before libuv poll returns to process nvim_input. In this case, nvim_get_mode will be processed in the same loop as nvim_input.

In that case IIUC nvim_get_mode() will return blocking=false, but the input will block. Right? Which means in that scenario (which hasn't been observed in the tests, yet) the worst that can happen is what already happens 100% of the time on current master.

my theory can be verified by making sure that the requests for nvim_input and nvim_get_mode are written to nvim pipe in a single batch(assuming the OS pipe buffer is large enough, which is probably true).

Confirmed. If nvim_get_mode is batched with a blocking nvim_input, it incorrectly returns blocked=false. Demonstrated via nvim_call_atomic:

36a7d3d#diff-5bf434f0a1fee647b4b121473e3410bfR280

It can't be irrelevant, because commenting out that line in input.c causes the tests to hang (at the nvim_get_mode RPC call--i.e. #6166).

Maybe because of the logic you added that makes nvim_get_mode not be an async call in some situations?

Maybe because of the logic you added that makes nvim_get_mode not be an async call in some situations?

Well, yes. And the purpose of that logic is to flush the input queue. If input_blocking() is false and there is pending input, asynchronous nvim_get_mode is useless.

Which means in that scenario (which hasn't been observed in the tests, yet) the worst that can happen is what already happens 100% of the time on current master.

True, but why add an API whose purpose is to test if the next call would block, but that has a possibility of returning wrong information? If the user can't rely on the return value 100% of times, he will probably not use it.

Even if you still want to add an API that allows the user to sometimes know if the next call would block, I think there's a much simpler way: You can implement a function marked as ASYNC that returns the state of the events_enabled variable.

Here's an example(patch to master):

diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c
index bf11795..fcd20a7 100644
--- a/src/nvim/api/vim.c
+++ b/src/nvim/api/vim.c
@@ -127,6 +127,12 @@ Integer nvim_input(String keys)
   return (Integer)input_enqueue(keys);
 }

+Boolean nvim_can_process_events(void)
+  FUNC_API_ASYNC
+{
+  return input_events_enabled();
+}
+
 /// Replaces any terminal codes with the internal representation
 ///
 /// @see replace_termcodes
diff --git a/src/nvim/os/input.c b/src/nvim/os/input.c
index 5f0f2ec..d454750 100644
--- a/src/nvim/os/input.c
+++ b/src/nvim/os/input.c
@@ -165,6 +165,11 @@ void input_disable_events(void)
   events_enabled--;
 }

+bool input_events_enabled(void)
+{
+  return events_enabled;
+}
+
 /// Test whether a file descriptor refers to a terminal.
 ///
 /// @param fd File descriptor.

@tarruda Does 303897b look right? Doesn't seem to work.

@tarruda Does 303897b look right? Doesn't seem to work.

I will have a look

@lunixbochs I changed the result of nvim_get_mode to be a dictionary:

{ "mode": String, "blocking": Boolean }