microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.46k stars 8.31k forks source link

Ctrl+C kills the process abruptly #18039

Open rom1v opened 2 weeks ago

rom1v commented 2 weeks ago

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): https://github.com/Genymobile/scrcpy/issues/5122#issuecomment-2243087702

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.

ronal33 commented 2 weeks ago

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 commented 1 week ago

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

rom1v commented 1 week ago

Thank you for your replies.

@carlos-zamora I could not reproduce myself.

@ronal33 I built a binary with your additional code, so that people could test: https://github.com/Genymobile/scrcpy/issues/5122#issuecomment-2408888597 But no answer so far.

zadjii-msft commented 1 week ago

I'm just gonna @vinayhatyal, since they're the user on your repo that originally filed this. Perhaps they can share some debugging light.

@vinayhatyal can you share the version of the Terminal that you're using? There's been some changes to the way process shutdown happens semi-recently, and I wonder if that was fixed since July

Zeroes1 commented 1 week ago

for first look all OK?

test under scrcpy-win64-v2.7.zip os 10.0.19045.3448 (22H2) release terminal-1.17.11461.0 and canary terminal-1.23.2913.0

under cmd and powershell 5.1 ran: scrcpy --record=file.mp4

and aftrer press Ctrl-C i see output to console: INFO: Recording complete to mp4 file: file.mp4

behavior as cmd (under conhost)

SetConsoleCtrlHandler is work?

Zeroes1 commented 5 days ago

@rom1v also i check (all what write up my post) with key --no-window all good too.

microsoft-github-policy-service[bot] commented 1 day ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.