Remove Nim signal handler for SIGINT (#25169)

Inside a signal handler, you cannot allocate memory because the signal
handler, being implemented with a C
[`signal`](https://en.cppreference.com/w/c/program/signal) call, can be
called _during_ a memory allocation - when that happens, the CTRL-C
handler causes a segfault and/or other inconsistent state.

Similarly, the call can happen from a non-nim thread or inside a C
library function call etc, most of which do not support reentrancy and
therefore cannot be called _from_ a signal handler.

The stack trace facility used in the default handler is unfortunately
beyond fixing without more significant refactoring since it uses
garbage-collected types in its API and implementation.

As an alternative to #25110, this PR
removes the most problematic signal handler, namely the one for SIGINT
(ctrl-c) - SIGINT is special because it's meant to cause a regular
shutdown of the application and crashes during SIGINT handling are both
confusing and, if turned into SIGSEGV, have downstream effects like core
dumps and OS crash reports.

The signal handlers for the various crash scenarios remain as-is - they
may too cause their own crashes but we're already going down in a bad
way, so the harm is more limited - in particular, crashing during a
crash handler corrupts `core`/crash dumps. Users wanting to keep their
core files pristine should continue to use `-d:noSignalHandler` - this
is usually the better option for production applications since they
carry more detail than the Nim stack trace that gets printed.

Finally, the example of a ctrl-c handler performs the same mistake of
calling `echo` which is not well-defined - replace it with an example
that is mostly correct (except maybe for the lack of `volatile` for the
`stop` variable).

Author: arnetheduck

lib/pure/cgi.nim

@@ -289,11 +289,14 @@ Content-Type: text/html
 proc writeErrorMessage*(data: string) =
   ## Tries to reset browser state and writes `data` to stdout in
   ## <plaintext> tag.
-  resetForStacktrace()
-  # We use <plaintext> here, instead of escaping, so stacktrace can
-  # be understood by human looking at source.
-  stdout.write("<plaintext>\n")
-  stdout.write(data)
+  try:
+    resetForStacktrace()
+    # We use <plaintext> here, instead of escaping, so stacktrace can
+    # be understood by human looking at source.
+    stdout.write("<plaintext>\n")
+    stdout.write(data)
+  except IOError as exc:
+    discard # Too bad..
 
 proc setStackTraceStdout*() =
   ## Makes Nim output stacktraces to stdout, instead of server log.

lib/system.nim

@@ -2194,18 +2194,33 @@ when not defined(js):
     when declared(initGC): initGC()
 
 when notJSnotNims:
-  proc setControlCHook*(hook: proc () {.noconv.})
+  proc setControlCHook*(hook: proc () {.noconv.}) {.raises: [], gcsafe.}
     ## Allows you to override the behaviour of your application when CTRL+C
     ## is pressed. Only one such hook is supported.
-    ## Example:
     ##
-    ##   ```nim
+    ## The handler runs inside a C signal handler and comes with similar
+    ## limitations.
+    ##
+    ## Allocating memory and interacting with most system calls, including using
+    ## `echo`, `string`, `seq`, raising or catching exceptions etc is undefined
+    ## behavior and will likely lead to application crashes.
+    ##
+    ## The OS may call the ctrl-c handler from any thread, including threads
+    ## that were not created by Nim, such as happens on Windows.
+    ##
+    ## ## Example:
+    ##
+    ## ```nim
+    ##   var stop: Atomic[bool]
     ##   proc ctrlc() {.noconv.} =
-    ##     echo "Ctrl+C fired!"
-    ##     # do clean up stuff
-    ##     quit()
+    ##     # Using atomics types is safe!
+    ##     stop.store(true)
     ##
     ##   setControlCHook(ctrlc)
+    ##
+    ##   while not stop.load():
+    ##     echo "Still running.."
+    ##     sleep(1000)
     ##   ```
 
   when not defined(noSignalHandler) and not defined(useNimRtl):

lib/system/ansi_c.nim

@@ -11,7 +11,7 @@
 # and definitions of Ansi C types in Nim syntax
 # All symbols are prefixed with 'c_' to avoid ambiguities
 
-{.push hints:off, stack_trace: off, profiler: off.}
+{.push hints:off, stack_trace: off, profiler: off, raises: [].}
 
 proc c_memchr*(s: pointer, c: cint, n: csize_t): pointer {.
   importc: "memchr", header: "<string.h>".}

lib/system/excpt.nim

@@ -17,7 +17,7 @@ const noStacktraceAvailable = "No stack traceback available\n"
 
 var
   errorMessageWriter*: (proc(msg: string) {.tags: [WriteIOEffect], benign,
-                                            nimcall.})
+                                            nimcall, raises: [].})
     ## Function that will be called
     ## instead of `stdmsg.write` when printing stacktrace.
     ## Unstable API.
@@ -58,6 +58,7 @@ proc showErrorMessage(data: cstring, length: int) {.gcsafe, raises: [].} =
       writeToStdErr(data, length)
 
 proc showErrorMessage2(data: string) {.inline.} =
+  # TODO showErrorMessage will turn it back to a string when a hook is set (!)
   showErrorMessage(data.cstring, data.len)
 
 proc chckIndx(i, a, b: int): int {.inline, compilerproc, benign.}
@@ -619,7 +620,7 @@ when not defined(noSignalHandler) and not defined(useNimRtl):
   type Sighandler = proc (a: cint) {.noconv, benign.}
     # xxx factor with ansi_c.CSighandlerT, posix.Sighandler
 
-  proc signalHandler(sign: cint) {.exportc: "signalHandler", noconv.} =
+  proc signalHandler(sign: cint) {.exportc: "signalHandler", noconv, raises: [].} =
     template processSignal(s, action: untyped) {.dirty.} =
       if s == SIGINT: action("SIGINT: Interrupted by Ctrl-C.\n")
       elif s == SIGSEGV:
@@ -641,6 +642,22 @@ when not defined(noSignalHandler) and not defined(useNimRtl):
     # print stack trace and quit
     when defined(memtracker):
       logPendingOps()
+    # On windows, it is common that the signal handler is called from a non-Nim
+    # thread and any allocation will (likely) cause a crash. Since we're about
+    # to quit, we can try setting up the GC - the correct course of action is to
+    # not use the GC at all in signal handlers but that requires redesigning
+    # the stack trace mechanism
+    when defined(windows):
+      when declared(initStackBottom):
+        initStackBottom()
+      when declared(initGC):
+        initGC()
+
+    # On other platforms, if memory needs to be allocated and the signal happens
+    # during memory allocation, we'll also (likely) see a crash and corrupt the
+    # memory allocator - less frequently than on windows but still.
+    # However, since we're about to go down anyway, YOLO.
+
     when hasSomeStackTrace:
       when not usesDestructors: GC_disable()
       var buf = newStringOfCap(2000)
@@ -653,14 +670,17 @@ when not defined(noSignalHandler) and not defined(useNimRtl):
       template asgn(y) =
         msg = y
       processSignal(sign, asgn)
-      # xxx use string for msg instead of cstring, and here use showErrorMessage2(msg)
-      # unless there's a good reason to use cstring in signal handler to avoid
-      # using gc?
+      # showErrorMessage may allocate, which may cause a crash, and calls C
+      # library functions which is undefined behavior, ie it may also crash.
+      # Nevertheless, we sometimes manage to emit the message regardless which
+      # pragmatically makes this attempt "useful enough".
+      # See also https://en.cppreference.com/w/c/program/signal
       showErrorMessage(msg, msg.len)
 
     when defined(posix):
       # reset the signal handler to OS default
-      c_signal(sign, SIG_DFL)
+      {.cast(raises: []).}: # Work around -d:laxEffects bugs
+        discard c_signal(sign, SIG_DFL)
 
       # re-raise the signal, which will arrive once this handler exit.
       # this lets the OS perform actions like core dumping and will
@@ -697,4 +717,5 @@ proc setControlCHook(hook: proc () {.noconv.}) =
 when not defined(noSignalHandler) and not defined(useNimRtl):
   proc unsetControlCHook() =
     # proc to unset a hook set by setControlCHook
-    c_signal(SIGINT, signalHandler)
+    {.gcsafe.}: # Work around -d:laxEffects bugs
+      discard c_signal(SIGINT, signalHandler)

lib/system/memtracker.nim

@@ -35,7 +35,7 @@ type
     count*: int
     disabled: bool
     data*: array[400, LogEntry]
-  TrackLogger* = proc (log: TrackLog) {.nimcall, tags: [], gcsafe.}
+  TrackLogger* = proc (log: TrackLog) {.nimcall, tags: [], gcsafe, raises: [].}
 
 var
   gLog*: TrackLog