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

[andeya]

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

Desktop (please complete the following information):

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

[davehorner]

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.

[davehorner]

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.

[davehorner]

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.

[davehorner]

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