Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse messages from guru on close_cb, not exit_cb #1439

Merged
merged 1 commit into from
Sep 12, 2017

Conversation

guns
Copy link
Contributor

@guns guns commented Sep 11, 2017

There is a race condition in async_guru in which not all messages from
a guru call are displayed:

  1. An async guru call is started.

  2. Lines streamed from guru are appended to a message list in
    callback().

  3. The guru process ends, exit_cb() is called, and the list of
    messages is processed for display.

  4. The rest of the lines from guru are appended to the message list.

Note that callback may still be called after exit_cb, as noted in
:help exit_cb:

"exit_cb": handler	…
			Note that data can be buffered,
			callbacks may still be called after
			the process ends.

This race was recently exposed by Vim v8.0.1073, which avoids a
redraw after statusline evaluation changes a highlight. Since vim-go
modifies the goStatusLineColor highlight definition on async calls, the
corresponding redraw acted as a delay, masking this data race.

The solution to this problem is to process guru output in close_cb,
which is called after all callback calls are complete.

There is a race condition in `async_guru` in which not all messages from
a `guru` call are displayed:

1. An async `guru` call is started.

2. Lines streamed from `guru` are appended to a message list in
   `callback()`.

3. The `guru` process ends, `exit_cb()` is called, and the list of
   messages is processed for display.

4. The rest of the lines from `guru` are appended to the message list.

Note that `callback` may still be called after `exit_cb`, as noted in
`:help exit_cb`:

	"exit_cb": handler	…
				Note that data can be buffered,
				callbacks may still be called after
				the process ends.

This race was recently exposed by [Vim v8.0.1073][vim], which avoids a
redraw after statusline evaluation changes a highlight. Since vim-go
modifies the goStatusLineColor highlight definition on async calls, the
corresponding redraw acted as a delay, masking this data race.

The solution to this problem is to process `guru` output in `close_cb`,
which is called after all `callback` calls are complete.

[vim]: vim/vim@ba2929b
@guns
Copy link
Contributor Author

guns commented Sep 11, 2017

The other async routines likely need to be fixed in the same manner, but I didn't look into it.

@fatih fatih merged commit eac41c9 into fatih:master Sep 12, 2017
@fatih
Copy link
Owner

fatih commented Sep 12, 2017

Thanks @guns 👍

@bhcleek
Copy link
Collaborator

bhcleek commented Sep 12, 2017

Is this likely to be the source of problems whereby Vim seems to not be repainting the screen?

I often find myself having to ctrl-l or even ctrl-c (which based on output in the statusline, is clearly is actually cancelling something) in order to make Vim usable again. Sometimes there just seems to be a small delay and sometimes Vim is mostly unresponsive until ctrl-l or ctrl-c. I mostly see this behavior in Normal mode. In some cases, the keys I'm pressing (e.g. j, k, etc.) are written to the screen (but disappear after ctrl-l) until a later time when they seem to be processed by Vim...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants