diff --git a/lib/pure/cgi.nim b/lib/pure/cgi.nim index 39fa25dcdc..3d5d4d932e 100644 --- a/lib/pure/cgi.nim +++ b/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 ## 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. diff --git a/lib/system.nim b/lib/system.nim index 646a301baa..76739e51f9 100644 --- a/lib/system.nim +++ b/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): diff --git a/lib/system/ansi_c.nim b/lib/system/ansi_c.nim index ed1a8aedf1..224354a848 100644 --- a/lib/system/ansi_c.nim +++ b/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>".} diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim index 5563b1adf0..a6a92ee127 100644 --- a/lib/system/excpt.nim +++ b/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) diff --git a/lib/system/memtracker.nim b/lib/system/memtracker.nim index 289f4e0245..518a1aa43b 100644 --- a/lib/system/memtracker.nim +++ b/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