From cfdac6666f5772479724e082fb08c2db694d8d40 Mon Sep 17 00:00:00 2001 From: Tail Wag Games Date: Fri, 29 Oct 2021 06:42:44 -0500 Subject: [PATCH] Freeing critical sections via atexit in system/alloc and system/io (#19062) * adding new system module sysexitprocs and including system exit procedures when registering exit handlers defined in userland * fixing failing tests and adding initialization guard to handle cases where the module's global init logic isn't invoked first as is the case with some gc implementaions * js backend shouldn't try to invoke actual system exit procs * fixing formatting in sysexitprocs.nim * 256 was too much - my max number of plugins in my engine is 64 and I require two hooks per runtime it looks like with tls emulation turned off, so for my purposes 128 should be sufficient * so atExit should be enough here, can get rid of all the extra cruft I had added on top since I didn't realize atExit already provided a stack * done being cute - since newruntime prevents correct cpp codegen for object variants apparently and breaks tests if I try to use std/exitprocs, ddSysExitProc is just going into both modules. Since system doesn't include system/io, polluting system with it doesn't make sense either... at least it is only importc'd when it is required in either module and we don't have to have any weird when defined(nimOwnedEnabled) with a comment explaining why --- lib/std/exitprocs.nim | 3 --- lib/system/alloc.nim | 3 +++ lib/system/io.nim | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/std/exitprocs.nim b/lib/std/exitprocs.nim index c76583a8ca1ab..c6537f7f89969 100644 --- a/lib/std/exitprocs.nim +++ b/lib/std/exitprocs.nim @@ -44,9 +44,6 @@ proc callClosures() {.noconv.} = of kClosure: fun.fun1() of kNoconv: fun.fun2() - when not defined(js) and not defined(nimOwnedEnabled): - deinitLock(gFunsLock) - template fun() = if gFuns.len == 0: addAtExit(callClosures) diff --git a/lib/system/alloc.nim b/lib/system/alloc.nim index 6d83a2c17875c..2c6ab4462cdd0 100644 --- a/lib/system/alloc.nim +++ b/lib/system/alloc.nim @@ -1054,9 +1054,12 @@ template instantiateForRegion(allocator: untyped) {.dirty.} = it = it.next when hasThreadSupport: + proc addSysExitProc(quitProc: proc() {.noconv.}) {.importc: "atexit", header: "".} + var sharedHeap: MemRegion var heapLock: SysLock initSysLock(heapLock) + addSysExitProc(proc() {.noconv.} = deinitSys(heapLock)) proc getFreeMem(): int = #sysAssert(result == countFreeMem()) diff --git a/lib/system/io.nim b/lib/system/io.nim index e7369392a6a92..2ad43acdbb755 100644 --- a/lib/system/io.nim +++ b/lib/system/io.nim @@ -777,15 +777,15 @@ proc setStdIoUnbuffered*() {.tags: [], benign.} = when declared(stdout): when defined(windows) and compileOption("threads"): + proc addSysExitProc(quitProc: proc() {.noconv.}) {.importc: "atexit", header: "".} + const insideRLocksModule = false include "system/syslocks" + var echoLock: SysLock initSysLock echoLock - - when not defined(js) and not defined(nimOwnedEnabled): - import std/exitprocs - addExitProc(proc() {.noconv.} = deinitSys echoLock) + addSysExitProc(proc() {.noconv.} = deinitSys(echoLock)) const stdOutLock = not defined(windows) and not defined(android) and