salvo-rs/salvo

The graceful shutdown function causes the process to hang when exiting.

Opened this issue · 4 comments

Describe the bug
After enabling graceful shutdown, the process often gets stuck when exiting and can only be resolved by forcefully killing the process.

Screenshots
screenshot-20240924-095102

Desktop (please complete the following information):

  • OS: Ubuntu and macOS
  • Version 0.72.3, 0.72.2 ...

I'm running on windows. I have also had problems with the server hanging on "salvo_core::server: wait for all connections to close" which never seems to return.

I have a endpoint that signals shutdown and I have tried both handle.stop_graceful(None) and handle.stop_forible(), I have also scheduled the shutdowns to occur after the endpoint returns without success.

I was going to attempt a time wait.

          if alive_connections.load(Ordering::Acquire) > 0 {
                tracing::info!("waiting for all connections to close.");
                tokio::select! {
                _ = notify.notified() => {
                    tracing::info!("All connections closed.");
                }
                _ = tokio::time::sleep(Duration::from_secs(5)) => {
                    tracing::warn!("Timeout waiting for connections to close.");
                }
                }
          }

The waiting occurs even when stop_forible() is called.

INFO salvo_core::server: force stop server
.
.
salvo_core::server: wait for all connections to close

I can work on a sample that demonstrates.

I forked and that works fine for testing modifications. I am able to force close and exit the app, but I am still holding the port open after termination when run from a detached process of the server. If I run it from cmd.exe separately, it does close the port. There's something about being detached from the server and asking the server to exit that causes issue for me.

I think the detached process is holding the socket open from inheritance.

use windows::Win32::Foundation::HANDLE;
use windows::Win32::System::Threading::{SetHandleInformation, HANDLE_FLAG_INHERIT};

fn disable_handle_inheritance(socket_handle: HANDLE) -> Result<()> {
    unsafe {
        SetHandleInformation(socket_handle, HANDLE_FLAG_INHERIT, 0);
    }
    Ok(())
}

the other thing to try is:

    // Set the `SO_REUSEADDR` option to allow reusing the address
    socket.set_reuse_address(true)?;

    // Optionally, you can set `SO_REUSEPORT` (if the platform supports it)
    // socket.set_reuse_port(true)?;

I'm not sure how to get the raw handle. I will keep trying.

I implemented

#[cfg(windows)]
impl AsRawSocket for TcpAcceptor {
    fn as_raw_socket(&self) -> std::os::windows::io::RawSocket {
        self.inner.as_raw_socket()
    }
}

and tried setting

    let raw_socket = ipv4_listener.as_raw_socket();
    println!("Raw socket handle: {}", raw_socket);
    use windows::Win32::Foundation::HANDLE;
    use windows::Win32::Foundation::{SetHandleInformation, HANDLE_FLAG_INHERIT};
    let raw_socket = std_listener.as_raw_socket();
    // // Cast the SOCKET (u64) to a HANDLE and set the handle information
    unsafe {
         let r=SetHandleInformation(HANDLE(raw_socket as *mut _), 0, HANDLE_FLAG_INHERIT);
         println!("{:?}",r);
    }

this is no good; as we need a HANDLE and not a SOCKET. I don't see a good safe way to do this. it's annoying inheritance is on by default on windows sockets.

at this point I have confirmed the inheritance is the problem for my hanging connection.

use windows::core::PWSTR;
use windows::Win32::System::Threading::{CreateProcessW, PROCESS_INFORMATION, STARTUPINFOW, DETACHED_PROCESS, STARTUPINFOW_FLAGS};
use windows::Win32::Foundation::CloseHandle;
use std::ffi::OsStr;
use std::os::windows::ffi::OsStrExt;

pub trait CommandExtDetached {
    fn spawn_detached(&mut self) -> std::io::Result<()>;
}

impl CommandExtDetached for Command {
    fn spawn_detached(&mut self) -> std::io::Result<()> {
        // Initialize the STARTUPINFOW structure
        let mut si: STARTUPINFOW = STARTUPINFOW::default();
        si.dwFlags = STARTUPINFOW_FLAGS(0);
        si.cb = std::mem::size_of::<STARTUPINFOW>() as u32;

        let mut pi: PROCESS_INFORMATION = PROCESS_INFORMATION::default();
                // Collect the program and args into Vec<String> to avoid moving the iterator
        let command = self.get_program();
        let args: Vec<String> = self.get_args().map(|arg| arg.to_string_lossy().to_string()).collect();
        
        // Create the full command line, similar to the working function
        let command_line = format!("{} {}", command.to_string_lossy(), args.join(" "));

        // Convert the command line string to a wide string (UTF-16)
        let mut command_line_wide: Vec<u16> = OsStr::new(&command_line)
            .encode_wide()
            .chain(Some(0))
            .collect(); // Null-terminate the wide string

        // Print out the command line for debugging
        println!("Command Line: {}", command_line);

        unsafe {
            let result = CreateProcessW(
                PWSTR::null(),                               // Application name (None here, using command line)
                PWSTR(command_line_wide.as_mut_ptr()),            // Command line (executable + arguments)
                None,                                        // Process security attributes
                None,                                        // Thread security attributes
                false,                                       // Inherit handles
                DETACHED_PROCESS,                            // Creation flags (detached process)
                None,                                        // Environment block
                None,                                        // Current directory
                &si,                                         // STARTUPINFO
                &mut pi,                                     // PROCESS_INFORMATION
            );

            if result.is_ok() {
                println!("Process created successfully");
                // Close handles to process and thread
                let _ = CloseHandle(pi.hProcess);
                let _ = CloseHandle(pi.hThread);
                Ok(())
            } else {
                eprintln!("Failed to create process: {:?}", result.err());
                Err(std::io::Error::new(std::io::ErrorKind::Other, "Failed to create process"))
            }
        }
    }
}


using the above I'm able to run commands that don't inherit the socket. I can run 'cargo run --release' and 'cargo run' and the two instances will close each other and grab 5800 accordingly. All my calls to Command::spawn() have to be replaced with spawn_detached() - not a huge deal.

maybe there's a better way. this works for me for now; maybe not a concern of salvo.

@andeya is on ubuntu and another concern most likely. @andeya can you say any more about what is going on when this occurs. from the cap it looks like https://github.com/neosmart/rsproxy?

I still think there might be a need for

                tokio::select! {
                _ = notify.notified() => {
                    tracing::info!("All connections closed.");
                }
                _ = tokio::time::sleep(Duration::from_secs(5)) => {
                    tracing::warn!("Timeout waiting for connections to close.");
                }
                }

I need to come up with a good way to test the graceful shutdowns and would like to understand how you're using rsproxy to experience your concern @andeya