Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

signal::windows::ctrl_close does not work as expected #6735

Open
sorokya opened this issue Jul 30, 2024 · 4 comments
Open

signal::windows::ctrl_close does not work as expected #6735

sorokya opened this issue Jul 30, 2024 · 4 comments
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-signal Module: tokio/signal

Comments

@sorokya
Copy link

sorokya commented Jul 30, 2024

Version
v1.39.2

Platform
64-bit (Windows)

Description
Tried writing some code for cross platform graceful shutdown in my project but the windows specific callbacks seem to not work.

I tried this code:

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    // snip
    tokio::select! {
        ctrl_c = signal::ctrl_c() => match ctrl_c {
            Ok(()) => {},
            Err(err) => {
                error!("Unable to listen for shutdown signal: {}", err);
            }
        },
        close = close() => match close {
            Ok(()) => {},
            Err(err) => {
                error!("Unable to listen for shutdown signal: {}", err);
            }
        }
    }

    info!("Shutting down server...");
    world.shutdown().await;

    Ok(())
}

#[cfg(windows)]
async fn close() -> Result<(), Box<dyn std::error::Error>> {
    let mut close_stream = signal::windows::ctrl_close()?;
    close_stream.recv().await;
    Ok(())
}

I expected to see this happen:
When I closed the console application my world.shutdown().await function would execute which notifies players of the shutdown and saves the server to the database.

Instead, this happened:
The process terminates before world.shutdown().await can run.

@sorokya sorokya added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Jul 30, 2024
@Darksonn Darksonn added M-process Module: tokio/process M-signal Module: tokio/signal and removed M-process Module: tokio/process labels Jul 30, 2024
@Darksonn
Copy link
Contributor

Thoughts @ipetkov ?

@dtzxporter
Copy link

dtzxporter commented Jul 30, 2024

I validated on windows that if you wish to do any closing work after receiving CTRL_CLOSE, you must do it in the handler before returning. The return value just stops calling handlers that are registered.

BOOL myHook(DWORD event) {
    // Say we handled it.
    std::cout << "handled" << std::endl;
    // If we don't sleep, process is killed immediately regardless of return value.
    // E.g, you must do your work in this callback.
    Sleep(5000);
    return TRUE; // True means stop calling handlers, False calls the next handler.
}

int main()
{
    SetConsoleCtrlHandler(myHook, TRUE);

    while (true) {
         std::cout << "Hello World!\n";
         Sleep(1000);
   }
}

The reason why this differs from ctrl_c is that the ctrl_close, the window is closed before the handlers are called, so the process is about to exit. ctrl_c doesn't close anything, and since we ignore the default handler, it doesn't force close the process there.

@ipetkov
Copy link
Member

ipetkov commented Jul 31, 2024

My understanding is/was that returning TRUE should stop other handlers from running including the default handler which would actually halt the process.

If that's not the case, then our implementation is incorrect

@Darksonn
Copy link
Contributor

Hmm. If this is the case, then I guess we'll need to deprecate ctrl_close with a message saying it doesn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-signal Module: tokio/signal
Projects
None yet
Development

No branches or pull requests

4 participants