llama.cpp icon indicating copy to clipboard operation
llama.cpp copied to clipboard

Handle signals properly on Windows

Open DannyDaemonic opened this issue 1 year ago • 2 comments

This uses Window's SetConsoleCtrlHandler to handle ctrl+c.

We currently use signal but Windows' official documentation for signal warns us:

  • Don't issue low-level or STDIO.H I/O routines (for example, printf or fread).
  • Don't call heap routines or any routine that uses the heap routines (for example, malloc, _strdup, or _putenv).
  • Don't use any function that generates a system call (for example, _getcwd or time).
  • Don't use longjmp unless the interrupt is caused by a floating-point exception (that is, sig is SIGFPE).
  • Don't use any overlay routines.

This wasn't a big deal before but since we now print timings on ctrl+c exit (1021), we should probably handle the signal properly.

The documentation for SetConsoleCtrlHandler has no such warning and their example code even shows them using printf.

Another advantage to doing it this way is we don't have to reinitialize SIGINT every loop in main like we currently do.

The only downside I see to this is the inclusion of <windows.h>, but fortunately we can cut down on what the header pulls in by defining WIN32_LEAN_AND_MEAN. If this still isn't satisfactory, I can change this to do a little cluster of defines at the top as is done in common.cpp.

DannyDaemonic avatar Apr 22 '23 11:04 DannyDaemonic

This is well-intentioned, but it seems we have fiddled a lot with the signal handler and the console state, just to get something that isn't even very user-friendly, see #1158.

As there doesn't seem to be a simple, portable way of doing non-blocking console input, maybe a solution could be to start a thread that simply uses blocking input functions.

We could then let Ctrl-C end the program without catching the signal. A disadvantage would be that colors wouldn't be restored.

That's not a vote against this PR, just throwing out this thought...

sw avatar Apr 25 '23 08:04 sw

I just figured if we were doing it, we should do it right.

I also agree it's a bit messy to use Ctrl-C to interrupt generation, but I would argue we're using Ctrl-C correctly to clean up the console and print stats as we're doing now. Both sigaction and SetConsoleCtrlHandler are safe, modern ways to handle signals.

Personally, I'd be happy to submit a patch that removes Ctrl-C as the means of interrupting generation. I know I've pressed it sometimes and then waited and waited, wondering if I really pressed it, only to press it again and have it kill the program. This can happen if you Ctrl-C right as it's resetting and rebuilding the context window. (It can take incredibly long to give you back control.)

As for doing this on another thread, I even mentioned handling input on another thread in #1040 (which has some console changes) under Future possible changes:

  • Read input on another thread
    • This would give us the ability to process each line as it is added as opposed to waiting until all input has been entered. This will also allow us to take input a little earlier as the model is being primed with " "
    • Would make the program "feel" a little faster

Since it processes one key at a time to allow us to clean up control characters such as \ (see the example images) it wouldn't be terribly hard to start a separate thread for input and allow anyone to just start typing to interrupt the generation.

DannyDaemonic avatar Apr 25 '23 10:04 DannyDaemonic