Merge branch 'devel' into pr_fix_ddc

Author: ringabout

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/pure/collections/lists.nim

@@ -712,6 +712,8 @@ proc remove*[T](L: var SinglyLinkedList[T], n: SinglyLinkedNode[T]): bool {.disc
     L.head = n.next
     if L.tail.next == n:
       L.tail.next = L.head # restore cycle
+    if L.tail == n:
+      L.tail = nil # reset tail if we removed the last node
   else:
     var prev {.cursor.} = L.head
     while prev.next != n and prev.next != nil:

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

tests/stdlib/tlists.nim

@@ -273,5 +273,17 @@ template main =
     list.add(n4)
     doAssert list.toSeq == @["sonic", "the", "hedgehog"]
 
+
+  block:
+    var list = initSinglyLinkedList[int]()
+
+    list.add(4)
+    list.remove(list.head)
+
+    list.add(5)
+    list.remove(list.head)
+
+    list.add(6)
+
 static: main()
 main()