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

Broken with neovim python-client 0.3.0: request from non-main thread #25

Closed
wookayin opened this issue Nov 4, 2018 · 5 comments · Fixed by #26
Closed

Broken with neovim python-client 0.3.0: request from non-main thread #25

wookayin opened this issue Nov 4, 2018 · 5 comments · Fixed by #26
Labels
bug Something isn't working

Comments

@wookayin
Copy link
Contributor

wookayin commented Nov 4, 2018

On startup After opening a python file, the following error occurs. Both on neovim 0.2.1 and 0.3.1.

request from non-main thread:
  File "$HOME/.dotfiles/vim/plugged/semshi/rplugin/python3/semshi/handler.py", line 242, in _update_hls
    self._add_hls(nodes_to_hl(add))
  File "$HOME/.dotfiles/vim/plugged/semshi/rplugin/python3/semshi/util.py", line 19, in wrapper
    res = func(*args, **kwargs)
  File "$HOME/.dotfiles/vim/plugged/semshi/rplugin/python3/semshi/handler.py", line 254, in _add_hls
    [('nvim_buf_add_highlight', (buf, *n)) for n in node_or_nodes])
  File "$HOME/.dotfiles/vim/plugged/semshi/rplugin/python3/semshi/handler.py", line 274, in _call_atomic_async
    self._vim.api.call_atomic(calls[i:i + batch_size], async_=True)

Installing neovim==0.2.6 is a workaround until it is fixed.
neovim API has breaking changes... See #25 (comment)

@numirias numirias added the bug Something isn't working label Nov 4, 2018
@numirias
Copy link
Owner

numirias commented Nov 4, 2018

Thanks for reporting! It seems with 0.3.0, Neovim is now raising an error when attempting to perform an API request from a different thread. (Although, technically it works.)

Since I'm currently traveling, I'll need some time to look into this and properly fix it. However, I'll try to come up with a quickfix that bypasses the error message until then. Sorry for the trouble.

(If someone feels like supplying a patch, please go ahead. I'm happy for a PR.)

@wookayin
Copy link
Contributor Author

wookayin commented Nov 4, 2018

Thanks for your response!

For other's reference, this is the commit that started to prevent requests from a different thread: neovim/pynvim#255

I will also look into this after I get into a free cycle.

@wookayin wookayin changed the title Broken with neovim python-client 0.3.0 Broken with neovim python-client 0.3.0: request from non-main thread Nov 5, 2018
@bfredl
Copy link

bfredl commented Nov 5, 2018

Calls from other threads must use nvim.async_call(closure), this has been documented for quite some time https://pynvim.readthedocs.io/en/latest/usage/python-plugin-api.html#async-calls.

It might have been working by accident for some use patterns, but is inherently racy and could trigger issues on any code or environmental change. An API should only be considered thread-safe it it explicitly claims to be. Changing unpredictable behavior to a consistent error message is not a breaking change, it only makes already present issues to be discovered faster.

@numirias
Copy link
Owner

numirias commented Nov 5, 2018

@bfredl Thanks for clarifying! Not sure how I missed that. This will probably resolve other bugs here too.

Mentioning in the error message or the source that async_call() should be used from threads might have made it even more obvious.

wookayin referenced this issue in wookayin/semshi Nov 6, 2018
Due to a change from neovim python-client 0.3.0, it is now required
that nvim API calls from other threads must use nvim.async_call().

This commit wraps all such API calls from the background thread
(via self._wrap_async) to make use of nvim.async_call().

See #25 for details.
numirias pushed a commit that referenced this issue Nov 6, 2018
Due to a change from neovim python-client 0.3.0, it is now required
that nvim API calls from other threads must use nvim.async_call().

This commit wraps all such API calls from the background thread
(via self._wrap_async) to make use of nvim.async_call().

See #25 for details.
@bfredl
Copy link

bfredl commented Nov 7, 2018

@numirias Indeed, I will improve the message to next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants