Make systray::Application amenable to being shared across threads

[qdot]

wait_for_message takes &mut self, which means we can't easily share the Application object between threads and also wait for menu selections. That makes systray less than useful for things like updating the icon without some sort of interaction (see https://github.com/qdot/systray-rs/issues/5#issuecomment-283553657).

Figure out a nice way to deal with sharing a mutable Application object between multiple threads. Maybe returning an Arc+Mutex wrapped object in the first place and passing that to a static equivalent of "wait_for_message" or something?

[qdot]

@jonhoo If you've got any good ideas for this, let me know. I'm still not super familiar with Rust yet so I'm not sure if there's a idiom I may be missing.

[jonhoo]

So, making it & instead of &mut doesn't actually help, because it still blocks the current thread when called, which precludes the application from doing anything to the Application while handling events. If Application was Sync (i.e., safe to access from multiple threads at once, which I don't know if is the case today?), then &Application is Send, so I could send off handles to the Application to another thread, which could then modify it at will. Unfortunately, Rust doesn't have scoped threads any more (thread closures must be 'static, which means they can't borrow anything), so even this doesn't quite work.

Another way to do it would be to have an ApplicationHandle type that is Clone (for example by wrapping an Arc<Application> as you mention). That way I could give one ApplicationHandle to my wait_for_message thread, and one to update the UI. I don't know if you then also need the Mutex (are gtk methods thread safe?), but if you do, that would break this. If a thread calls wait_for_message, it would take the Mutex, preventing any other thread from updating the UI :/ Is there a way you could have wait_for_message only take the lock when an event actually occurs?

[qdot]

I think I can do this by making the event receive channel member of the struct public. That way, we can just clone it and don't have to access the object itself while running .recv(), solving the event locking problem. It also gives users more choice in where to put that, in case they want to pool it with other channels or something.

[jonhoo]

Why would it need to be public? Having access to an &mut of the channel or something might be okay, but then you also need a mechanism to pass the received events back into Application so that the right callbacks are called. Also, the channel still wouldn't be able to pass between threads, so you're then forcing the user to use channels and the unstable select feature :/

[qdot]

Ok, well, for now I can at least figure out a way to remove the dependency on &mut self/&self in wait_for_message (which I also need to rename to just something like run_message_loop heh) and we can continue from there, just to get a bit more functionality going. I've got a test case for changing an icon in another thread that I'm trying to make work, once I get something going for that I'll put it up here for review.

I didn't plan on the channel being passed between threads, I was more thinking that if you already had a main thread that waits on other things, waiting on that event channel can be pooled with those things. Didn't realize select was unstable though. Hmm.

[jonhoo]

Yeah, it is really unfortunate that select is unstable. See https://github.com/rust-lang/rust/issues/27800. And it doesn't seem like there's been any progress on it lately. Forcing the user to handle events on another thread is annoying, I agree, but it might be the best you can do until select stabilizes :(

[bbigras]

Is it possible right now to update the icon while wait_for_message() is looping?

[bbigras]

In the mean time I'm using a custom wait_for_message().

   pub fn try_wait_for_message(&mut self) -> bool {
        match self.rx.try_recv() {
            Ok(m) => {
                if (*self.callback.borrow()).contains_key(&m.menu_index) {
                    let f = (*self.callback.borrow_mut()).remove(&m.menu_index).unwrap();
                    f(&self);
                    (*self.callback.borrow_mut()).insert(m.menu_index, f);
                }
                false
            },
            Err(e) => {
                match e {
                    std::sync::mpsc::TryRecvError::Disconnected => {
                        if let Some(t) = self.windows_loop.take() {
                            t.join().ok();
                        }

                        true
                    },
                    std::sync::mpsc::TryRecvError::Empty => false,
                }
            },
        }
    }

[qdot]

@bbigras That /should/ be a thing, right now it's just not because I didn't architect this well. Hoping to get back to finishing this up soon, with things like that in mind.

[Ciantic]

I did trayicon API (trayicon-rs) with sender idea, so that you can plug your own sender (e.g. winit's EventLoop, or std::mpsc::channel::Sender) and the trayicon does not have main loop. It works great e.g. with winit because it inits main loop already. I didn't want my trayicon to have main loop.