From 254fbcc548247865cf15200a200436024258e647 Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Thu, 23 Mar 2017 17:13:38 +0200 Subject: [PATCH] Fixes #4719. (#5585) --- lib/system/alloc.nim | 3 +++ lib/system/threads.nim | 42 ++++++++++++++++++++---------------- tests/threads/tmanyjoin.nim | 30 ++++++++++++++++++++++++++ tests/threads/treusetvar.nim | 28 ++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 tests/threads/tmanyjoin.nim create mode 100644 tests/threads/treusetvar.nim diff --git a/lib/system/alloc.nim b/lib/system/alloc.nim index cde17bead..edd8ececb 100644 --- a/lib/system/alloc.nim +++ b/lib/system/alloc.nim @@ -763,6 +763,8 @@ proc getOccupiedMem(a: MemRegion): int {.inline.} = # ---------------------- thread memory region ------------------------------- template instantiateForRegion(allocator: untyped) = + {.push stackTrace: off.} + when defined(fulldebug): proc interiorAllocatedPtr*(p: pointer): pointer = result = interiorAllocatedPtr(allocator, p) @@ -850,5 +852,6 @@ template instantiateForRegion(allocator: untyped) = proc getOccupiedSharedMem(): int = sharedMemStatsShared(sharedHeap.currMem - sharedHeap.freeMem) + {.pop.} {.pop.} diff --git a/lib/system/threads.nim b/lib/system/threads.nim index 5cdca5470..dc9cc9656 100644 --- a/lib/system/threads.nim +++ b/lib/system/threads.nim @@ -242,7 +242,6 @@ type PGcThread = ptr GcThread GcThread {.pure, inheritable.} = object - sys: SysThread when emulatedThreadVars and not useStackMaskHack: tls: ThreadLocalStorage else: @@ -345,18 +344,16 @@ when not defined(useNimRtl): # use ``stdcall`` since it is mapped to ``noconv`` on UNIX anyway. type - Thread* {.pure, final.}[TArg] = - object of GcThread ## Nim thread. A thread is a heavy object (~14K) - ## that **must not** be part of a message! Use - ## a ``ThreadId`` for that. + Thread* {.pure, final.}[TArg] = object + core: PGcThread + sys: SysThread when TArg is void: dataFn: proc () {.nimcall, gcsafe.} else: dataFn: proc (m: TArg) {.nimcall, gcsafe.} data: TArg - ThreadId*[TArg] = ptr Thread[TArg] ## the current implementation uses - ## a pointer as a thread ID. -{.deprecated: [TThread: Thread, TThreadId: ThreadId].} + +{.deprecated: [TThread: Thread].} var threadDestructionHandlers {.rtlThreadVar.}: seq[proc () {.closure, gcsafe.}] @@ -423,19 +420,20 @@ proc threadProcWrapStackFrame[TArg](thrd: ptr Thread[TArg]) = when declared(threadType): threadType = ThreadType.NimThread when declared(registerThread): - thrd.stackBottom = addr(thrd) - registerThread(thrd) + thrd.core.stackBottom = addr(thrd) + registerThread(thrd.core) p(thrd) - when declared(registerThread): unregisterThread(thrd) + when declared(registerThread): unregisterThread(thrd.core) when declared(deallocOsPages): deallocOsPages() else: threadProcWrapDispatch(thrd) template threadProcWrapperBody(closure: expr) {.immediate.} = - when declared(globalsSlot): threadVarSetValue(globalsSlot, closure) + var thrd = cast[ptr Thread[TArg]](closure) + var core = thrd.core + when declared(globalsSlot): threadVarSetValue(globalsSlot, thrd.core) when declared(initAllocator): initAllocator() - var thrd = cast[ptr Thread[TArg]](closure) threadProcWrapStackFrame(thrd) # Since an unhandled exception terminates the whole process (!), there is # no need for a ``try finally`` here, nor would it be correct: The current @@ -444,7 +442,9 @@ template threadProcWrapperBody(closure: expr) {.immediate.} = # page! # mark as not running anymore: + thrd.core = nil thrd.dataFn = nil + deallocShared(cast[pointer](core)) {.push stack_trace:off.} when defined(windows): @@ -502,6 +502,10 @@ when false: discard pthread_cancel(t.sys) when declared(registerThread): unregisterThread(addr(t)) t.dataFn = nil + ## if thread `t` already exited, `t.core` will be `null`. + if not isNil(t.core): + deallocShared(t.core) + t.core = nil when hostOS == "windows": proc createThread*[TArg](t: var Thread[TArg], @@ -510,9 +514,11 @@ when hostOS == "windows": ## creates a new thread `t` and starts its execution. Entry point is the ## proc `tp`. `param` is passed to `tp`. `TArg` can be ``void`` if you ## don't need to pass any data to the thread. + t.core = cast[PGcThread](allocShared0(sizeof(GcThread))) + when TArg isnot void: t.data = param t.dataFn = tp - when hasSharedHeap: t.stackSize = ThreadStackSize + when hasSharedHeap: t.core.stackSize = ThreadStackSize var dummyThreadId: int32 t.sys = createThread(nil, ThreadStackSize, threadProcWrapper[TArg], addr(t), 0'i32, dummyThreadId) @@ -532,9 +538,11 @@ else: ## creates a new thread `t` and starts its execution. Entry point is the ## proc `tp`. `param` is passed to `tp`. `TArg` can be ``void`` if you ## don't need to pass any data to the thread. + t.core = cast[PGcThread](allocShared0(sizeof(GcThread))) + when TArg isnot void: t.data = param t.dataFn = tp - when hasSharedHeap: t.stackSize = ThreadStackSize + when hasSharedHeap: t.core.stackSize = ThreadStackSize var a {.noinit.}: PthreadAttr pthread_attr_init(a) pthread_attr_setstacksize(a, ThreadStackSize) @@ -554,10 +562,6 @@ else: proc createThread*(t: var Thread[void], tp: proc () {.thread, nimcall.}) = createThread[void](t, tp) -proc threadId*[TArg](t: var Thread[TArg]): ThreadId[TArg] {.inline.} = - ## returns the thread ID of `t`. - result = addr(t) - when false: proc mainThreadId*[TArg](): ThreadId[TArg] = ## returns the thread ID of the main thread. diff --git a/tests/threads/tmanyjoin.nim b/tests/threads/tmanyjoin.nim new file mode 100644 index 000000000..2c1cda494 --- /dev/null +++ b/tests/threads/tmanyjoin.nim @@ -0,0 +1,30 @@ +discard """ + outputsub: "129" +""" + +import os, locks + +type + MarkerObj = object + lock: Lock + counter: int + Marker = ptr MarkerObj + +const + ThreadsCount = 129 + SleepTime = 1000 + +proc worker(p: Marker) {.thread.} = + acquire(p.lock) + inc(p.counter) + release(p.lock) + sleep(SleepTime) + +var p = cast[Marker](allocShared0(sizeof(MarkerObj))) +initLock(p.lock) +var ts = newSeq[Thread[Marker]](ThreadsCount) +for i in 0..