lemunozm / message-io

Fast and easy-to-use event-driven network library.
Apache License 2.0
1.11k stars 74 forks source link

Pending task after CTRL+C #117

Closed fgadaleta closed 2 years ago

fgadaleta commented 2 years ago

In main I start two tasks via tokio. One is a graphql server and the other is a service running node_listener.for_each(... to handle messages

    // Create and start discovery service
    // thread::spawn(move || {
    let t1 = tokio::task::spawn_blocking(move || {
        debug!("Creating Discovery Service");
        let dsserver = DiscoveryServer::new();
        if let Ok(discovery) = dsserver {
            debug!("Starting discovery service");
            let _ = discovery.run();
        }
    });

    tokio::join!(async move {
        let _ = graphql_server.run().await;
    });

When I want to terminate, the discovery service stays and can only be killed brutally sudo kill -9 PID Is there a clean way to structure the code such that it can terminate gently?

lemunozm commented 2 years ago

Hi.

It seems the internal thread is still running when you do ctrl+c. You should call NodeHandler::stop() before. To perform clean exits when the user makes ctrl+c I recommend https://github.com/Detegr/rust-ctrlc to handle the signal before the OS kill the process

fgadaleta commented 2 years ago

Ok but if i un DiscoverServer in a tokio task, I also ctrlc::set_handler(... in main outside of that task. How would I access discovery or dsserver ?

lemunozm commented 2 years ago

This depends on how your architecture is built. I would send an event that the task could receive. Also, the NodeHandler is clonable and sendable, so you could store any copy of them where you want to then call .stop() before the program exits.

fgadaleta commented 2 years ago

indeed i clone the entire DiscoveryServer (that implements run() and stop())

This is what I do in main https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=3ed82949b6852827866aaf5ee6cc07cf

I run the server in a tokio task and i catch a ctrlc signal from main and call stop() from the cloned Arc<Mutex>

I see a signal has been catched but nothing is actually stopped

lemunozm commented 2 years ago

I checked the code and it seems the mutex is locked at line 29, before you perform the stop() called. Could you try to move the println() after this line and check if it's printed?

fgadaleta commented 2 years ago

yes i already tried that and indeed it prints Mutex { data: <locked>, poisoned: false, .. }

fgadaleta commented 2 years ago

the discovery.run() does something like

    pub fn run(&mut self) -> std::io::Result<()> {
        let node_listener = self.node_listener.take().unwrap();

        node_listener.for_each(move |event| match event.network() {
...
...
lemunozm commented 2 years ago

If I understand well the code you pasted and you shared now, your run() call will keep processing forever (until you call stop()). During this processing, the mutex is locked, so, when you try to lock again to make the stop() call, it is already locked. If your problem is getting the NodeHandler under the mutex, remember that you can clone it, and then use the cloned NodeHandler without making the lock()

fgadaleta commented 2 years ago

Exactly, run() runs forever. How can i call stop() directly if that's behind a Mutex ?

lemunozm commented 2 years ago

A solution could be:

  1. Create your DiscoveryServer instance.
  2. get the NodeHandler from the discovery server and clone it.
  3. Create the mutex over the discovery server.
  4. lock() and call run().
  5. use the node_handler.stop() (which is out of the mutex) at ctrl_c signal.
lemunozm commented 2 years ago

You only need to clone NodeHandler which is already clonable, you do not need to clone the entire DiscoveryServer.

fgadaleta commented 2 years ago

lol it worked! It's a bit ugly but thank you so much!!