daa84 / neovim-lib

Rust library for Neovim clients
GNU Lesser General Public License v3.0
192 stars 26 forks source link

Api and Mutability/Ownership deadlock #16

Closed SephVelut closed 6 years ago

SephVelut commented 6 years ago

I have a handler which I pass along to Session start_event_loop_handler(handler). Event loop spinning. Want to subscribe to certain events so I call subscribe on Neovim, after starting the event loop.

One problem. I want the handler to use functions like get_current_buffer. It needs a &mut Neovim. No problem, would normally construct the handler with that reference to the Neovim binding as its field. Here's the issue though, I need to be able to still use that Neovim binding after I've given it to a handler AND I need to allow multiple handlers to use that same mutable reference to make other NeomvimApi calls since most of those functions accept a &mut Neovim. Here it is

let mut neovim = Neovim::new(Session::new_tcp("127.0.0.1:6666").unwrap());
let handler = SomeHandler {neovim: &mut neovim};

// Mutability exclusion violations
neovim.session.start_event_loop_handler(handler);
neovim.subscribe("get_generics").expect("cannot subscribe to event: get_generics");

Yeah I could subscribe and start an event loop before I deal with the handler but I want to be able subscribe to events long after I start handling others. I tried something using Arc<Mutex<Neovim>> and got it to compile, unfortunately that did something "bad" because non of the NeovimApi methods I try to call work (they all timeout). This is the best neovim rpc crate and I like it but it seem like I'm kind of stuck, based on how the api is laid out here. Handling - Subscribing - Handling -Subscribing in any order but at least handling first before subscribing so you don't miss events doesn't sound like a far out use case does it?

daa84 commented 6 years ago

There is two threads: 1. main 2. event_loop All events come from nvim will be processed in event_loop. If you stop processing event_loop (for example by block it with mutex) you get timeout error. So good solution from my point of view is to make event_loop just pass events data to main thread and do nothing about actual event processing. This way it will always work and you don't need to pass Neovim instance to another thread.

So there is number of possible solutions to make it work. I know two:

  1. You can use rust channels to pass events data from event_loop thread to main thread and do all processing in main thread - here example of test plugin: https://github.com/boxofrox/neovim-scorched-earth
  2. Solution that i use in neovim-gtk - again pass all events to main thread but with special gtk_idle function
SephVelut commented 6 years ago

Ah the timeout error makes perfect sense now. I'm using a channel as solution right now but I can also just spawn some threads for handling and it seems to work. Is this documented somewhere? If not I think it should definitely be mentioned. This is a potential deadlock without timeout in place to error, so a warning with a /// # Deadlock comment on start_event_loop_* I think is needed.

daa84 commented 6 years ago

I think it will be good to write some high level api for writing plugins with some simple example.

SephVelut commented 6 years ago

The thing I am concerned about is that calling the NeovimApi methods from a Handler seems like a natural tendency. But since that spawns some commands to be executed by the event loop running in the exact same thread - to the unweary - it will cause an automatic soft or hard deadlock.