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

Ctrl+C kills the process abruptly #18039

Open
rom1v opened this issue Oct 12, 2024 · 2 comments
Open

Ctrl+C kills the process abruptly #18039

rom1v opened this issue Oct 12, 2024 · 2 comments
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@rom1v
Copy link

rom1v commented Oct 12, 2024

Windows Terminal version

No response

Windows build number

No response

Other Software

No response

Steps to reproduce

Hello,

In a program called scrcpy that I develop, I listen to Ctrl+C on Windows using SetConsoleCtrlHandler(windows_ctrl_handler, TRUE);:

https://github.com/Genymobile/scrcpy/blob/665ccb32f5306ebd866dc0d99f4d08ed2aeb91c3/app/src/scrcpy.c#L148-L149

It works correctly in the default (old) Windows console (typically started by cmd.exe), Ctrl+C is catched and the program can terminate gracefully (finishing writing a video footer for example).

However, from Windows Terminal, it has been reported that Ctrl+C terminates the process abruptly (which in that case cause the recorded mp4 file to be corrupted, but any other cleanup actions cannot be executed): Genymobile/scrcpy#5122 (comment)

Is there a reason for this behavior?

Thank you for your help.

Expected Behavior

The Ctrl handler should be executed on Ctrl+C.

Actual Behavior

The process is killed abruptly on Ctrl+C.

@rom1v rom1v added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 12, 2024
@ronal33
Copy link

ronal33 commented Oct 13, 2024

Could you add this code?
when this flag is added, ctrl+C is processed as a control event and the handler set with SetConsoleCtrlHandler is sent

static void
sdl_configure(bool video_playback, bool disable_screensaver) {
#ifdef _WIN32
    // Clean up properly on Ctrl+C on Windows
    bool ok = SetConsoleCtrlHandler(windows_ctrl_handler, TRUE);
    if (!ok) {
        LOGW("Could not set Ctrl+C handler");
    }

    // Habilitar ENABLE_PROCESSED_INPUT
    HANDLE hStdin = GetStdHandle(STD_INPUT_HANDLE);
    DWORD mode;
    if (GetConsoleMode(hStdin, &mode)) {
        mode |= ENABLE_PROCESSED_INPUT;
        if (!SetConsoleMode(hStdin, mode)) {
            LOGW("Could not set console mode");
        }
    } else {
        LOGW("Could not get console mode");
    }
#endif // _WIN32

    if (!video_playback) {
        return;
    }

    if (disable_screensaver) {
        SDL_DisableScreenSaver();
    } else {
        SDL_EnableScreenSaver();
    }
}

@carlos-zamora
Copy link
Member

Thanks for filing! Could you create a minimal repro in a single file?

@carlos-zamora carlos-zamora added Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Repro We can't figure out how to make this happen. Please help find a simplified repro. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Development

No branches or pull requests

3 participants