asyncdispatch+stackTraceOverride: fix premature garbage collection

changelog.md

@@ -310,6 +310,7 @@
 
 - Added `copyWithin` [for `seq` and `array` for JavaScript targets](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/copyWithin).
 
+- Fixed premature garbage collection in asyncdispatch, when a stack trace override is in place.
 
 ## Language changes
 

lib/pure/asyncfutures.nim

@@ -278,19 +278,33 @@ proc `callback=`*[T](future: Future[T],
   ## If future has already completed then `cb` will be called immediately.
   future.callback = proc () = cb(future)
 
+template getFilenameProcname(entry: StackTraceEntry): (string, string) =
+  when compiles(entry.filenameStr) and compiles(entry.procnameStr):
+    # We can't rely on "entry.filename" and "entry.procname" still being valid
+    # cstring pointers, because the "string.data" buffers they pointed to might
+    # be already garbage collected (this entry being a non-shallow copy,
+    # "entry.filename" no longer points to "entry.filenameStr.data", but to the
+    # buffer of the original object).
+    (entry.filenameStr, entry.procnameStr)
+  else:
+    ($entry.filename, $entry.procname)
+
 proc getHint(entry: StackTraceEntry): string =
   ## We try to provide some hints about stack trace entries that the user
   ## may not be familiar with, in particular calls inside the stdlib.
+
+  let (filename, procname) = getFilenameProcname(entry)
+
   result = ""
-  if entry.procname == cstring"processPendingCallbacks":
-    if cmpIgnoreStyle(entry.filename, "asyncdispatch.nim") == 0:
+  if procname == "processPendingCallbacks":
+    if cmpIgnoreStyle(filename, "asyncdispatch.nim") == 0:
       return "Executes pending callbacks"
-  elif entry.procname == cstring"poll":
-    if cmpIgnoreStyle(entry.filename, "asyncdispatch.nim") == 0:
+  elif procname == "poll":
+    if cmpIgnoreStyle(filename, "asyncdispatch.nim") == 0:
       return "Processes asynchronous completion events"
 
-  if entry.procname.endsWith(NimAsyncContinueSuffix):
-    if cmpIgnoreStyle(entry.filename, "asyncmacro.nim") == 0:
+  if procname.endsWith(NimAsyncContinueSuffix):
+    if cmpIgnoreStyle(filename, "asyncmacro.nim") == 0:
       return "Resumes an async procedure"
 
 proc `$`*(stackTraceEntries: seq[StackTraceEntry]): string =
@@ -303,16 +317,20 @@ proc `$`*(stackTraceEntries: seq[StackTraceEntry]): string =
   # Find longest filename & line number combo for alignment purposes.
   var longestLeft = 0
   for entry in entries:
-    if entry.procname.isNil: continue
+    let (filename, procname) = getFilenameProcname(entry)
+
+    if procname == "": continue
 
-    let left = $entry.filename & $entry.line
-    if left.len > longestLeft:
-      longestLeft = left.len
+    let leftLen = filename.len + len($entry.line)
+    if leftLen > longestLeft:
+      longestLeft = leftLen
 
   var indent = 2
   # Format the entries.
   for entry in entries:
-    if entry.procname.isNil:
+    let (filename, procname) = getFilenameProcname(entry)
+
+    if procname == "":
       if entry.line == reraisedFromBegin:
         result.add(spaces(indent) & "#[\n")
         indent.inc(2)
@@ -321,11 +339,11 @@ proc `$`*(stackTraceEntries: seq[StackTraceEntry]): string =
         result.add(spaces(indent) & "]#\n")
       continue
 
-    let left = "$#($#)" % [$entry.filename, $entry.line]
+    let left = "$#($#)" % [filename, $entry.line]
     result.add((spaces(indent) & "$#$# $#\n") % [
       left,
       spaces(longestLeft - left.len + 2),
-      $entry.procname
+      procname
     ])
     let hint = getHint(entry)
     if hint.len > 0:
@@ -349,9 +367,9 @@ proc injectStacktrace[T](future: Future[T]) =
     newMsg.add($entries)
 
     newMsg.add("Exception message: " & exceptionMsg & "\n")
-    newMsg.add("Exception type:")
 
     # # For debugging purposes
+    # newMsg.add("Exception type:")
     # for entry in getStackTraceEntries(future.error):
     #   newMsg.add "\n" & $entry
     future.error.msg = newMsg

lib/system/exceptions.nim

@@ -29,7 +29,7 @@ type
       programCounter*: uint ## Program counter - will be used to get the rest of the info,
                             ## when `$` is called on this type. We can't use
                             ## "cuintptr_t" in here.
-      procnameStr*, filenameStr*: string ## GC-ed objects holding the cstrings in "procname" and "filename"
+      procnameStr*, filenameStr*: string ## GC-ed alternatives to "procname" and "filename"
 
   Exception* {.compilerproc, magic: "Exception".} = object of RootObj ## \
     ## Base exception class.

tests/async/tasync_traceback.nim

@@ -86,7 +86,7 @@ Async traceback:
     asyncfutures\.nim\(\d+?\)\s+?read
   \]#
 Exception message: b failure
-Exception type:
+
 
 bar failure
 Async traceback:
@@ -114,7 +114,7 @@ Async traceback:
     asyncfutures\.nim\(\d+?\)\s+?read
   \]#
 Exception message: bar failure
-Exception type:
+
 """
 
 # TODO: is asyncmacro good enough location for fooIter traceback/debugging? just put the callsite info for all?