run destructors for .threadvars on thread exit

compiler/backend/backends.nim

@@ -216,11 +216,30 @@ proc generateTeardown*(graph: ModuleGraph, modules: ModuleList, result: PNode) =
   for it in rclosed(modules):
     if sfSystemModule notin it.sym.flags:
       emitOpCall(graph, it.destructor, result)
+      emitOpCall(graph, it.threadDestructor, result)
       emitOpCall(graph, it.postDestructor, result)
 
+  # the destructor calls for procedure-scoped threadvars are also part of the
+  # postDestructor module op, meaning that the threadPostDestructor op must
+  # not be invoked
+
   emitOpCall(graph, systemModule(modules).destructor, result)
+  emitOpCall(graph, systemModule(modules).threadDestructor, result)
   emitOpCall(graph, systemModule(modules).postDestructor, result)
 
+proc generateThreadTeardown*(graph: ModuleGraph, modules: ModuleList,
+                             result: PNode) =
+  ## Generates the code for de-initializing all threadvars for the program,
+  ## and emits it to `result`. Destruction order is the same as for non-
+  ## threadlocal state.
+  for it in rclosed(modules):
+    if sfSystemModule notin it.sym.flags:
+      emitOpCall(graph, it.threadDestructor, result)
+      emitOpCall(graph, it.threadPostDestructor, result)
+
+  emitOpCall(graph, systemModule(modules).threadDestructor, result)
+  emitOpCall(graph, systemModule(modules).threadPostDestructor, result)
+
 proc generateMainProcedure*(graph: ModuleGraph, idgen: IdGenerator,
                             modules: ModuleList): PSym =
   ## Generates the procedure for initializing, running, and de-initializing
@@ -375,7 +394,7 @@ proc generateIR*(graph: ModuleGraph, idgen: IdGenerator, env: var MirEnv,
 
 proc produceFragmentsForGlobals(
     env: var MirEnv, identdefs: seq[PNode], graph: ModuleGraph,
-    config: TranslationConfig): tuple[init, deinit: MirBody] =
+    config: TranslationConfig): tuple[init, deinit, threadDeinit: MirBody] =
   ## Given a list of identdefs of lifted globals, produces the MIR code for
   ## initialzing and deinitializing the globals. All not-yet-seen globals and
   ## threadvars are added to `env`.
@@ -395,7 +414,7 @@ proc produceFragmentsForGlobals(
     # we're creating a body here, so there is no list of locals yet
     result = finish(bu, default(Store[LocalId, Local]))
 
-  var init, deinit: MirBuilder
+  var init, deinit, threadDeinit: MirBuilder
 
   # lifted globals can appear re-appear in the identdefs list for two reasons:
   # - the definition appears in the body of a for-loop using an inline iterator
@@ -406,27 +425,32 @@ proc produceFragmentsForGlobals(
   # only want to generate code for the first definition we encounter
   for it in identdefs.items:
     let s = it[0].sym
-    # threadvars don't support initialization nor destruction, so they're
-    # skipped
-    if sfThread in s.flags:
-      discard env.globals.add(s)
-    elif s notin env.globals: # cull duplicates
+    if s notin env.globals: # cull duplicates
       let global = env.globals.add(s)
-      # generate the MIR code for an initializing assignment:
-      prepare(init, result.init.source, graph.emptyNode)
-      generateAssignment(graph, env, config, it, init, result.init.source)
+      if sfThread notin s.flags:
+        # generate the MIR code for an initializing assignment:
+        prepare(init, result.init.source, graph.emptyNode)
+        generateAssignment(graph, env, config, it, init, result.init.source)
+
+      template destroyOp(bu: var MirBuilder, sm: var SourceMap) =
+        prepare(bu, sm, graph.emptyNode)
+        bu.setSource(sm.add(it[0]))
+        genDestroy(bu, graph, env, toValue(global, env.types.add(s.typ)))
 
       # if the global requires one, emit a destructor call into the deinit
       # fragment:
       if hasDestructor(s.typ):
-        prepare(deinit, result.deinit.source, graph.emptyNode)
-        deinit.setSource(result.deinit.source.add(it[0]))
-        genDestroy(deinit, graph, env, toValue(global, env.types.add(s.typ)))
+        destroyOp(deinit, result.deinit.source)
+        if sfThread in s.flags:
+          # also emit a destructor into the thread-deinit fragment:
+          destroyOp(threadDeinit, result.threadDeinit.source)
 
   (result.init.code, result.init.locals) =
     finish(init, result.init.source, graph.emptyNode)
   (result.deinit.code, result.deinit.locals) =
     finish(deinit, result.deinit.source, graph.emptyNode)
+  (result.threadDeinit.code, result.threadDeinit.locals) =
+    finish(threadDeinit, result.threadDeinit.source, graph.emptyNode)
 
 # ----- dynlib handling -----
 
@@ -713,7 +737,8 @@ iterator process*(graph: ModuleGraph, modules: var ModuleList,
   # mark all procedures that require incremental code generation as forwarded,
   # so that they're not queued for normal code generation
   for _, m in modules.modules.pairs:
-    for it in [m.preInit, m.postDestructor, m.dynlibInit]:
+    for it in [m.preInit, m.postDestructor, m.threadPostDestructor,
+               m.dynlibInit]:
       it.flags.incl sfForward
 
   discovery.progress = checkpoint(env)
@@ -731,6 +756,9 @@ iterator process*(graph: ModuleGraph, modules: var ModuleList,
     if not isTrivialProc(graph, m.destructor):
       discard env.procedures.add(m.destructor)
 
+    if not isTrivialProc(graph, m.threadDestructor):
+      discard env.procedures.add(m.threadDestructor)
+
     # register the globals and threadvars:
     for s in m.structs.globals.items:
       discard env.globals.add(s)
@@ -811,11 +839,12 @@ iterator process*(graph: ModuleGraph, modules: var ModuleList,
       queue.prepend(module, WorkItem(kind: wikReportConst, cnst: item.cnst))
     of wikProcessGlobals:
       # produce the init/de-init code for the lifted globals:
-      let (init, deinit) =
+      let (init, deinit, threadDeinit) =
         produceFragmentsForGlobals(env, item.globals, graph, conf.tconfig)
 
       pushProgress(modules[module].preInit, init, module)
       pushProgress(modules[module].postDestructor, deinit, module)
+      pushProgress(modules[module].threadPostDestructor, threadDeinit, module)
     of wikImported:
       let id = item.imported
       # the procedure is always reported from the module its attached to
@@ -841,7 +870,8 @@ iterator process*(graph: ModuleGraph, modules: var ModuleList,
 
   # unmark all completed incremental procedures:
   for _, m in modules.modules.pairs:
-    for it in [m.preInit, m.postDestructor, m.dynlibInit]:
+    for it in [m.preInit, m.postDestructor, m.threadPostDestructor,
+               m.dynlibInit]:
       if not isTrivialProc(graph, it):
         it.flags.excl sfForward
 

compiler/backend/cbackend.nim

@@ -366,6 +366,27 @@ proc generateCodeForMain(m: BModule, modules: ModuleList) =
   # XXX: ^^ this is going to change in the future
   genMainProc(m, code)
 
+proc generateThreadTeardown(m: BModule, modules: ModuleList) =
+  ## Generates and emits the C code for the ``nimTeardownThreadVars``
+  ## procedure.
+  let body = newNode(nkStmtList)
+  generateThreadTeardown(m.g.graph, modules, body)
+
+  let p = newProc(nil, m)
+  p.flags.incl nimErrorFlagDisabled
+  p.options = {}
+  p.body = canonicalize(m.g.graph, m.idgen, m.g.env, m.module, body,
+                        TranslationConfig())
+
+  # manually produced the C code for the procedure:
+  genStmts(p, p.body.code)
+  var code = "void nimTeardownThreadVars(void) {\n"
+  code.add(p.s(cpsLocals))
+  code.add(p.s(cpsInit))
+  code.add(p.s(cpsStmts))
+  code.add "}\n"
+  m.s[cfsProcs].add code
+
 proc generateCode*(graph: ModuleGraph, g: BModuleList, mlist: sink ModuleList)
 
 proc generateCode*(graph: ModuleGraph, mlist: sink ModuleList) =
@@ -499,6 +520,8 @@ proc generateCode*(graph: ModuleGraph, g: BModuleList, mlist: sink ModuleList) =
     if sfMainModule in m.sym.flags:
       finalizeMainModule(bmod)
       generateCodeForMain(bmod, mlist)
+      if optThreads in graph.config.globalOptions:
+        generateThreadTeardown(bmod, mlist)
 
     # code generation for the module is done; its C code will not change
     # anymore beyond this point

compiler/sem/modulelowering.nim

@@ -71,8 +71,11 @@ type
     init*: PSym
       ## the procedure responsible for initializing the module's globals
     destructor*: PSym
-      ## the prodcedure responsible for de-initializing the module's
+      ## the procedure responsible for de-initializing the module's
       ## globals
+    threadDestructor*: PSym
+      ## the procedure responsible for de-initializing the module's
+      ## thread-local variables
 
     # XXX: the design around the pre-init and post-destructor procedure is
     #      likely not final yet. At the moment, we set them up here so that
@@ -81,6 +84,8 @@ type
       ## the procedure for initializing the module's lifted globals
     postDestructor*: PSym
       ## the procedure for destroying the module's lifted globals
+    threadPostDestructor*: PSym
+      ## the procedure for destroying the module's lifted threadvars
     dynlibInit*: PSym
       ## the procedure for loading the dynamic libraries, procedure, and
       ## variables associated with the module
@@ -299,12 +304,13 @@ proc genDestroy(graph: ModuleGraph, dest: PNode): PNode =
 
   result = newTreeI(nkCall, dest.info, newSymNode(op), addrExp)
 
-proc generateModuleDestructor(graph: ModuleGraph, m: Module): PNode =
-  ## Generates the body for the destructor procedure of module `m` (also
-  ## referred to as the 'de-init' procedure).
+proc generateDestructor(graph: ModuleGraph, vars: openArray[PSym]): PNode =
+  ## Generates the body for a module destructor (also referred to as the
+  ## 'de-init' procedure). A destructor call for each entitiy in `vars` is
+  ## emitted, in reverse order of appearance.
   result = newNode(nkStmtList)
-  for i in countdown(m.structs.globals.high, 0):
-    let s = m.structs.globals[i]
+  for i in countdown(vars.high, 0):
+    let s = vars[i]
     if hasDestructor(s.typ):
       result.add genDestroy(graph, newSymNode(s))
 
@@ -414,11 +420,22 @@ proc setupModule*(graph: ModuleGraph, idgen: IdGenerator, m: PSym,
   result.dataInit = createModuleOp(graph, idgen, "DatInit", m, newNode(nkEmpty), options)
 
   # setup the module struct clean-up operator:
-  let destructorBody = generateModuleDestructor(graph, result)
-  result.destructor = createModuleOp(graph, idgen, "Deinit", m, destructorBody, options)
+  result.destructor =
+    createModuleOp(graph, idgen, "Deinit", m,
+                   generateDestructor(graph, result.structs.globals),
+                   options)
+
+  # setup the per-thread module struct clean-up operator:
+  result.threadDestructor =
+    createModuleOp(graph, idgen, "ThreadDeinit", m,
+                   generateDestructor(graph, result.structs.threadvars),
+                   options)
 
   result.preInit = createModuleOp(graph, idgen, "PreInit", m, newNode(nkEmpty), options)
   result.postDestructor = createModuleOp(graph, idgen, "PostDeinit", m, newNode(nkEmpty), options)
+  result.threadPostDestructor =
+    createModuleOp(graph, idgen, "ThreadPostDeinit", m, newNode(nkEmpty),
+                   options)
   result.dynlibInit = createModuleOp(graph, idgen, "DynlibInit", m, newNode(nkEmpty), options)
 
 # Below is the `passes` interface implementation

lib/system/threads.nim

@@ -119,6 +119,10 @@ proc deallocOsPages() {.rtl, raises: [].}
 proc threadTrouble() {.raises: [], gcsafe.}
   ## defined in system/excpt.nim
 
+proc nimTeardownThreadVars() {.noconv, importc: "nimTeardownThreadVars".}
+  ## Generated by the compiler. Runs the destructor for every thread-local
+  ## variable.
+
 when true:
   proc threadProcWrapDispatch[TArg](thrd: ptr ThreadCore[TArg]) {.raises: [].} =
     try:
@@ -130,6 +134,7 @@ when true:
       threadTrouble()
     finally:
       afterThreadRuns()
+      nimTeardownThreadVars()
 
 template threadProcWrapperBody(closure: untyped): untyped =
   let core = cast[ptr ThreadCore[TArg]](closure)

tests/threads/mthreadvars_destruction.nim

@@ -0,0 +1,15 @@
+type Object* = object
+  val*: int
+
+proc `=destroy`(x: var Object) =
+  if x.val != 0:
+    echo x.val
+
+let global = Object(val: 7)
+var tv1 {.threadvar.}: Object
+tv1 = Object(val: 8)
+
+proc init*() =
+  tv1 = Object(val: 3)
+  var tv2 {.threadvar.}: Object
+  tv2 = Object(val: 4)

tests/threads/tthreadvars_destruction.nim

@@ -0,0 +1,31 @@
+discard """
+  description: '''
+    Ensure that threadvars are destroyed and that the order is correct:
+    1. inter-module: the module closed last has threadvars destroyed first
+    2. intra-module:
+      i. top-level threadvars are destroyed, in reverse order of definition
+      ii. threadvars defined within routines are destroyed, in an
+         unspecified order
+
+    For the main thread, module-level threadvars are destroyed after top-level
+    thread-globals but before procedure-scoped threadvars and thread-globals.
+  '''
+  output: "1\n2\n3\n4\n5\n6\n7\n8\n"
+"""
+
+import mthreadvars_destruction
+
+# the threadvars and globals from this module are destoyed first
+let global = Object(val: 5)
+var tv1 {.threadvar.}: Object
+tv1 = Object(val: 6)
+
+proc run() {.thread.} =
+  tv1 = Object(val: 1)
+  var tv2 {.threadvar.}: Object
+  tv2 = Object(val: 2)
+
+  init()
+
+var t = (createThread[void])(run)
+t.joinThread()