Graceful shutdown and cleanup

[Michael-F-Bryan]

It's kinda annoying how the websockets library has a push-style of doing things (it fires off callbacks when things happen) instead of a pull-style (think iterator of messages) because making sure the listener thread shuts down gracefully when we want it to becomes a pain...

One possible way of doing things is to spin off the listener thread, passing an Arc<AtomicBool> (or Arc<Mutex<bool>>) to the Client which can function as a running flag.

Then in a Connection's shutdown method it'll set the running flag to false and wait on the listener thread's handle. For convenience you'd probably also make Connection's drop impl call shutdown() automatically.

Meanwhile on the Client side you can set a timeout when it first connects. When the timeout fires, Client will wake up and can check if it's still running, re-scheduling the timeout if it is and closing the connection otherwise.

That's all awfully convoluted though, so hopefully you can come up with a less hacky/racy solution 😜

[ttdonovan]

@Michael-F-Bryan I must not be understanding what it is you're saying... I'm attempting to setup a send_ch on the Connection but failing to understand how to get the rx to receive messages for the Client's out.

Branch: ttdonovan/connection-send_ch
Diff: master...ttdonovan/connection-send_ch

[Michael-F-Bryan]

I actually implemented a version of this before creating the issue, but wasn't satisfied with it and wanted to see if you had a better solution. I believe the "move out of FnMut" issue is because your connect() method actually contains two closures, yet you're only putting move on the inner one.

Also, the Client will still need to contain a ws::Sender because that's what it uses to read and write messages and set the wakeup-and-check-running timeout "hack".

[ttdonovan]

Ok I'm really struggling tonight and might just need to take a break - if I remove the move then I get a compiler error about not being able to share the std::sync::mpsc::Reciever<()>.

error[E0277]: the trait bound `std::sync::mpsc::Receiver<()>: std::marker::Sync` is not satisfied
  --> src/connection.rs:63:26
   |
63 |                         .spawn(|| {
   |                          ^^^^^ `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Receiver<()>`
   = note: required because it appears within the type `[closure@src/connection.rs:63:32: 76:26 send_rx:&std::sync::mpsc::Receiver<()>]`

I've posted a message on the user form asking for feedback and maybe someone will have a better solution.

[ttdonovan]

@Michael-F-Bryan I've made another attempt at setting up the multiple channels and use of threads but instead this time with rust-websocket crate. Still not working and I might be confusing myself with not needed threads and stuck in a loop.

7b5e684

I'm starting to think now only one thread is need for the "client" connection (currently named recev_thread) and the Connection recv_response() and send_request() will use the send_txandsend_rx` channels. I may take another try at this later tonight.

[Michael-F-Bryan]

Just looking at rust-websocket's examples, I reckon their pull-style API will be a lot easier to work with. When I get home tonight I'll give it a shot.

Isn't the API set up so that each bot needs its own connection to the server?

[ttdonovan]

OK I think I may understand what I need to do now - I created a pseudo example but do not have SC2 running to test it with websockets. What I believe is needed are 3 channels and 2 threads.

1. WebSocket client channels client_tx and client_rx
2. Receiving thread and channels (recv_tx, recv_rx) that listens for messages from client_tx.incoming_messages()
3. Sending thread and channels (send_tx, send_rx) that sends messages to client_rx.send_message()
4. A Connection { recv_ch: recv_rx, send_ch: send_tx ...}
5. A fn resv_response that uses recv_ch.recv()
6. A fn send_request that uses send_ch.send()

f14dc6f

For some reason I really struggled wrapping my head around how to set this up - I hope I'm right...

[ttdonovan]

TODO: look into https://crates.io/crates/tungstenite