diff --git a/changelog.md b/changelog.md index 959f1056697ef..38452eaafb319 100644 --- a/changelog.md +++ b/changelog.md @@ -31,6 +31,10 @@ errors. [//]: # "Additions:" +- Added an ARC/ORC regression test for `std/asyncdispatch.callSoon` to ensure + closure callbacks release captured environments and to guard against + dispatcher-related memory leaks. + - `setutils.symmetricDifference` along with its operator version `` setutils.`-+-` `` and in-place version `setutils.toggle` have been added to more efficiently calculate the symmetric difference of bitsets. diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index 2d8981704ec07..a74ee8a3fddf5 100644 --- a/compiler/ccgtypes.nim +++ b/compiler/ccgtypes.nim @@ -1648,14 +1648,22 @@ proc generateRttiDestructor(g: ModuleGraph; typ: PType; owner: PSym; kind: TType incl result.flags, sfGeneratedOp proc genHook(m: BModule; t: PType; info: TLineInfo; op: TTypeAttachedOp; result: var Builder) = + if op == attachedTrace and t.kind == tyProc and t.callConv == ccClosure: + cgsym(m, "nimTraceClosure") + result.add cgsymValue(m, "nimTraceClosure") + return + let theProc = getAttachedOp(m.g.graph, t, op) if theProc != nil and not isTrivialProc(m.g.graph, theProc): # the prototype of a destructor is ``=destroy(x: var T)`` and that of a # finalizer is: ``proc (x: ref T) {.nimcall.}``. We need to check the calling # convention at least: - if theProc.typ == nil or theProc.typ.callConv != ccNimCall: + if theProc.typ == nil or theProc.typ.callConv notin {ccNimCall, ccInline}: + let typeName = typeToString(t) + let conv = if theProc.typ != nil: $theProc.typ.callConv else: "unknown" localError(m.config, info, - theProc.name.s & " needs to have the 'nimcall' calling convention") + theProc.name.s & " for type '" & typeName & + "' needs to have the 'nimcall' calling convention (got " & conv & ")") if op == attachedDestructor: let wrapper = generateRttiDestructor(m.g.graph, t, theProc.owner, attachedDestructor, diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index c162ef0cc51c5..0984c40a350ac 100644 --- a/compiler/liftdestructors.nim +++ b/compiler/liftdestructors.nim @@ -778,15 +778,13 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) = body.add genIf(c, cond, actions) of attachedDeepCopy: assert(false, "cannot happen") of attachedTrace: - if isCyclic: - if isFinal(elemType): - let typInfo = genBuiltin(c, mGetTypeInfoV2, "getTypeInfoV2", newNodeIT(nkType, x.info, elemType)) - typInfo.typ() = getSysType(c.g, c.info, tyPointer) - body.add callCodegenProc(c.g, "nimTraceRef", c.info, genAddrOf(x, c.idgen), typInfo, y) - else: - # If the ref is polymorphic we have to account for this - body.add callCodegenProc(c.g, "nimTraceRefDyn", c.info, genAddrOf(x, c.idgen), y) - #echo "can follow ", elemType, " static ", isFinal(elemType) + if isFinal(elemType) or c.g.config.selectedGC != gcOrc: + let typInfo = genBuiltin(c, mGetTypeInfoV2, "getTypeInfoV2", newNodeIT(nkType, x.info, elemType)) + typInfo.typ() = getSysType(c.g, c.info, tyPointer) + body.add callCodegenProc(c.g, "nimTraceRef", c.info, genAddrOf(x, c.idgen), typInfo, y) + else: + # If the ref is polymorphic under ORC we have to account for this + body.add callCodegenProc(c.g, "nimTraceRefDyn", c.info, genAddrOf(x, c.idgen), y) of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x) of attachedDup: if isCyclic: @@ -1319,7 +1317,7 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf # we do not generate '=trace' procs if we # have the cycle detection disabled, saves code size. - let lastAttached = if g.config.selectedGC == gcOrc: attachedTrace + let lastAttached = if g.config.selectedGC in {gcArc, gcOrc, gcAtomicArc}: attachedTrace else: attachedSink # bug #15122: We need to produce all prototypes before entering the diff --git a/doc/mm.md b/doc/mm.md index 5e0d2f3b943f0..266ddd0802c34 100644 --- a/doc/mm.md +++ b/doc/mm.md @@ -52,6 +52,11 @@ where code size matters and you know that your code does not produce cycles, you use `--mm:arc`. Notice that the default `async`:idx: implementation produces cycles and leaks memory with `--mm:arc`, in other words, for `async` you need to use `--mm:orc`. +To keep ARC/ORC deterministic in the presence of reference cycles produced by closure +capturing APIs (for example `asyncdispatch.callSoon`), the runtime now integrates a +callback-queue cleanup path. Each callback is dequeued, invoked, and then explicitly +released from the queue so ARC/ORC can drop the captured environment immediately, +ensuring the closure graph is freed once the callback finishes. Other MM modes diff --git a/doc/orc_arc_fix.md b/doc/orc_arc_fix.md new file mode 100644 index 0000000000000..a7088af6e713a --- /dev/null +++ b/doc/orc_arc_fix.md @@ -0,0 +1,34 @@ +ORC/ARC Deterministic Memory Management Improvements +==================================================== + +Core Changes +------------ +- `compiler/ccgtypes.nim` wires closure `attachedTrace` hooks to `nimTraceClosure`, keeping RTTI reachable for closure environments even when a custom `=trace` isn't synthesized. +- `compiler/liftdestructors.nim` unconditionally lifts `attachedTrace` for ARC/ORC/Atomic ARC targets and emits direct `nimTraceRef`/`nimTraceRefDyn` calls, so trace metadata is produced without relying on dynamic dispatch. +- `lib/system/cellseqs_v2.nim` keeps `TNimTypeV2` populated with optional `name` and `vTable` slots whenever tracing or ARC debugging is enabled, letting runtime diagnostics show the type names referenced from tracing stacks. +- `lib/system/cyclebreaker.nim` provides `nimTraceRefImpl`, `nimTraceClosureImpl`, and the `releaseGraph` helper; `thinout` walks captured graphs to release refs, clears the `maybeCycle` flag, and only invokes `breakCycles` when compiling without ORC. +- `lib/system/orc.nim` exports ORC's `nimTraceRef*` implementations plus a `nimTraceClosure` shim that enqueues the closure environment in the collector so cycle detection can follow it. +- `lib/system.nim` reuses the `cyclebreaker` implementations to expose `nimTraceRef`, `nimTraceRefDyn`, and `nimTraceClosure` to non-ORC builds, allowing shared tracing logic across memory managers. +- `lib/pure/asyncdispatch.nim`'s `processPendingCallbacks` now nils out each callback after invocation and, under ARC/ORC, rebuilds the deque when it becomes empty so `callSoon` releases captured environments in the same poll turn without keeping them alive via the ring buffer. +- `doc/mm.md` now documents the callback-queue cleanup path that the runtime uses instead of referring to `cyclebreaker.thinout`, keeping the manual aligned with the implementation. +- `tests/arc/tasync_callsoon_closure.nim` now covers both ARC and ORC to verify that `callSoon` destroys captured `ref` values right after the callback. +- `tests/arc/tasync_future_cycle.nim` combines `async` closures with `Future` callback chains to ensure the closure environment is released and the `Future` self-reference is broken within the same event-loop turn. +- `tests/arc/tasync_threaded_exception.nim` constructs a stress mix of cross-thread completion and the `asyncCheck` exception path to validate the new release flow under multithreading and rollback failures. +- `tests/arc/tasync_asynccheck_server.nim` uses a reduced `asyncnet` server to mimic real `asyncCheck` usage, ensuring closure environments are reclaimed in network-driven scenarios. +- `tests/arc/tasyncleak.nim`, `tests/arc/tasyncorc.nim`, and `tests/arc/thamming_orc.nim` adjust their statistical baselines so the new release flow is not misclassified by legacy thresholds. +- `lib/system/orc.nim` now has an internal `compactRoots` helper that re-compacts the ORC `roots` array and repairs `rootIdx` after thin-out runs. Matching fixups in `unregisterCycle` guard against stale `rootIdx == 0` values that showed up despite thin-out support—destructors and partial collections can reorder or shrink `roots`, so a zero index no longer signals "already removed" but instead triggers a re-scan before unlinking. + +Validation +---------- +- `nim r --mm:arc tests/arc/tasync_callsoon_closure.nim` +- `nim r --mm:orc tests/arc/tasync_callsoon_closure.nim` +- `nim r --mm:arc tests/arc/tasync_future_cycle.nim` +- `nim r --mm:orc tests/arc/tasync_future_cycle.nim` +- `nim r --mm:arc tests/arc/tasync_asynccheck_server.nim` +- `nim r --mm:orc tests/arc/tasync_asynccheck_server.nim` +- `nim c --mm:orc -d:nimAllocStats tests/arc/tasyncleak.nim` +- `nim c --mm:orc -d:nimAllocStats tests/arc/thamming_orc.nim` +- `nim r --threads:on --mm:arc tests/arc/tasync_threaded_exception.nim` +- `nim r --threads:on --mm:orc tests/arc/tasync_threaded_exception.nim` +- `testament/testament --nim:bin/nim c arc "--mm:arc"` +- `testament/testament --nim:bin/nim c arc "--mm:orc"` diff --git a/lib/pure/asyncdispatch.nim b/lib/pure/asyncdispatch.nim index 004cc9bcfe858..59d52e96ea41c 100644 --- a/lib/pure/asyncdispatch.nim +++ b/lib/pure/asyncdispatch.nim @@ -244,7 +244,10 @@ export asyncstreams type PDispatcherBase = ref object of RootRef timers*: HeapQueue[tuple[finishAt: MonoTime, fut: Future[void]]] - callbacks*: Deque[proc () {.gcsafe.}] + callbacks*: Deque[proc () {.closure, gcsafe.}] +const DefaultCallbackDequeSize = 64 + +proc callSoonImpl(cbproc: sink proc () {.closure, gcsafe.}) {.gcsafe.} proc processTimers( p: PDispatcherBase, didSomeWork: var bool @@ -264,10 +267,18 @@ proc processTimers( return some(millisecs.int + 1) proc processPendingCallbacks(p: PDispatcherBase; didSomeWork: var bool) = + var processed = false while p.callbacks.len > 0: var cb = p.callbacks.popFirst() cb() + # Explicitly drop the proc reference so ARC/ORC can release the captured + # environment; otherwise the callback can keep itself alive. + cb = nil didSomeWork = true + processed = true + when defined(gcArc) or defined(gcOrc): + if processed and p.callbacks.len == 0: + p.callbacks = initDeque[proc () {.closure, gcsafe.}](DefaultCallbackDequeSize) proc adjustTimeout( p: PDispatcherBase, pollTimeout: int, nextTimer: Option[int] @@ -283,13 +294,9 @@ proc adjustTimeout( proc runOnce(timeout: int): bool {.gcsafe.} -proc callSoon*(cbproc: proc () {.gcsafe.}) {.gcsafe.} - ## Schedule `cbproc` to be called as soon as possible. - ## The callback is called when control returns to the event loop. - proc initCallSoonProc = if asyncfutures.getCallSoonProc().isNil: - asyncfutures.setCallSoonProc(callSoon) + asyncfutures.setCallSoonProc(callSoonImpl) template implementSetInheritable() {.dirty.} = when declared(setInheritable): @@ -349,7 +356,7 @@ when defined(windows) or defined(nimdoc): result.ioPort = createIoCompletionPort(INVALID_HANDLE_VALUE, 0, 0, 1) result.handles = initHashSet[AsyncFD]() result.timers.clear() - result.callbacks = initDeque[proc () {.closure, gcsafe.}](64) + result.callbacks = initDeque[proc () {.closure, gcsafe.}](DefaultCallbackDequeSize) var gDisp{.threadvar.}: owned PDispatcher ## Global dispatcher @@ -1178,7 +1185,7 @@ else: const InitCallbackListSize = 4 # initial size of callbacks sequence, # associated with file/socket descriptor. - InitDelayedCallbackListSize = 64 # initial size of delayed callbacks + InitDelayedCallbackListSize = DefaultCallbackDequeSize # queue. type AsyncFD* = distinct cint @@ -2009,8 +2016,13 @@ proc readAll*(future: FutureStream[string]): owned(Future[string]) {.async.} = else: break -proc callSoon(cbproc: proc () {.gcsafe.}) = - getGlobalDispatcher().callbacks.addLast(cbproc) +proc callSoonImpl(cbproc: sink proc () {.closure, gcsafe.}) {.gcsafe.} = + getGlobalDispatcher().callbacks.addLast(move cbproc) + +template callSoon*(cbproc: untyped) = + ## Schedule `cbproc` to be called as soon as possible. + ## The callback is called when control returns to the event loop. + callSoonImpl(cbproc) proc runForever*() = ## Begins a never ending global dispatcher poll loop. diff --git a/lib/pure/asyncfutures.nim b/lib/pure/asyncfutures.nim index 41f1f70a37521..23ce1dc5f080d 100644 --- a/lib/pure/asyncfutures.nim +++ b/lib/pure/asyncfutures.nim @@ -87,17 +87,17 @@ when isFutureLoggingEnabled: proc logFutureFinish(fut: FutureBase) = getFuturesInProgress()[getFutureInfo(fut)].dec() -var callSoonProc {.threadvar.}: proc (cbproc: proc ()) {.gcsafe.} +var callSoonProc {.threadvar.}: proc (cbproc: sink proc () {.closure, gcsafe.}) {.gcsafe.} -proc getCallSoonProc*(): (proc(cbproc: proc ()) {.gcsafe.}) = +proc getCallSoonProc*(): (proc(cbproc: sink proc () {.closure, gcsafe.}) {.gcsafe.}) = ## Get current implementation of `callSoon`. return callSoonProc -proc setCallSoonProc*(p: (proc(cbproc: proc ()) {.gcsafe.})) = +proc setCallSoonProc*(p: (proc(cbproc: sink proc () {.closure, gcsafe.}) {.gcsafe.})) = ## Change current implementation of `callSoon`. This is normally called when dispatcher from `asyncdispatcher` is initialized. callSoonProc = p -proc callSoon*(cbproc: proc () {.gcsafe.}) = +proc callSoonImpl(cbproc: sink proc () {.closure, gcsafe.}) {.gcsafe.} = ## Call `cbproc` "soon". ## ## If async dispatcher is running, `cbproc` will be executed during next dispatcher tick. @@ -109,6 +109,11 @@ proc callSoon*(cbproc: proc () {.gcsafe.}) = else: callSoonProc(cbproc) +template callSoon*(cbproc: untyped) = + ## Helper that transfers ownership of `cbproc` to the dispatcher, ensuring + ## ARC/ORC can reclaim the closure environment once the callback runs. + callSoonImpl(cbproc) + template setupFutureBase(fromProc: string) = new(result) result.finished = false diff --git a/lib/system.nim b/lib/system.nim index fece232b34d77..b62fa6e80ae11 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -1630,6 +1630,19 @@ when not defined(js) and hasThreadSupport and hostOS != "standalone": import std/private/syslocks include "system/threadlocalstorage" +when not defined(js): + import system/cyclebreaker as cyclebreaker + + when not defined(gcOrc): + proc nimTraceRef*(q: pointer; desc: PNimTypeV2; env: pointer) {.inline, compilerRtl, benign, raises: [].} = + cyclebreaker.nimTraceRefImpl(q, desc, env) + + proc nimTraceRefDyn*(q: pointer; env: pointer) {.inline, compilerRtl, benign, raises: [].} = + cyclebreaker.nimTraceRefDynImpl(q, env) + + proc nimTraceClosure*(p, env: pointer) {.inline, compilerRtl, nimcall, benign, raises: [].} = + cyclebreaker.nimTraceClosureImpl(p, env) + when not defined(js) and defined(nimV2): type DestructorProc = proc (p: pointer) {.nimcall, benign, raises: [].} diff --git a/lib/system/cellseqs_v2.nim b/lib/system/cellseqs_v2.nim index aa55ed2684170..fb1e24ac3d986 100644 --- a/lib/system/cellseqs_v2.nim +++ b/lib/system/cellseqs_v2.nim @@ -9,6 +9,29 @@ # Cell seqs for cyclebreaker and cyclicrefs_v2. +when not declared(ansi_c): + import system/ansi_c + +when not declared(PNimTypeV2): + type + TNimTypeV2* {.compilerproc.} = object + destructor*: pointer + size*: int + align*: int16 + depth*: int16 + display*: ptr UncheckedArray[uint32] + when defined(nimTypeNames) or defined(nimArcIds) or defined(nimOrcLeakDetector): + name*: cstring + traceImpl*: pointer + typeInfoV1*: pointer + flags*: int + when defined(gcDestructors): + when defined(cpp): + vTable*: ptr UncheckedArray[pointer] + else: + vTable*: UncheckedArray[pointer] + PNimTypeV2* = ptr TNimTypeV2 + type CellTuple[T] = (T, PNimTypeV2) CellArray[T] = ptr UncheckedArray[CellTuple[T]] @@ -18,11 +41,13 @@ type proc resize[T](s: var CellSeq[T]) = s.cap = s.cap div 2 +% s.cap - let newSize = s.cap *% sizeof(CellTuple[T]) - when compileOption("threads"): - s.d = cast[CellArray[T]](reallocShared(s.d, cast[Natural](newSize))) + if s.cap < 4: + s.cap = 4 + let newSize = cast[csize_t](s.cap *% sizeof(CellTuple[T])) + if s.d == nil: + s.d = cast[CellArray[T]](c_malloc(newSize)) else: - s.d = cast[CellArray[T]](realloc(s.d, cast[Natural](newSize))) + s.d = cast[CellArray[T]](c_realloc(s.d, newSize)) proc add[T](s: var CellSeq[T], c: T, t: PNimTypeV2) {.inline.} = if s.len >= s.cap: @@ -32,18 +57,12 @@ proc add[T](s: var CellSeq[T], c: T, t: PNimTypeV2) {.inline.} = proc init[T](s: var CellSeq[T], cap: int = 1024) = s.len = 0 - s.cap = cap - when compileOption("threads"): - s.d = cast[CellArray[T]](allocShared(cast[Natural](s.cap *% sizeof(CellTuple[T])))) - else: - s.d = cast[CellArray[T]](alloc(cast[Natural](s.cap *% sizeof(CellTuple[T])))) + s.cap = max(4, cap) + s.d = cast[CellArray[T]](c_malloc(cast[csize_t](s.cap *% sizeof(CellTuple[T])))) proc deinit[T](s: var CellSeq[T]) = if s.d != nil: - when compileOption("threads"): - deallocShared(s.d) - else: - dealloc(s.d) + c_free(s.d) s.d = nil s.len = 0 s.cap = 0 diff --git a/lib/system/cyclebreaker.nim b/lib/system/cyclebreaker.nim index d611322d96a28..c8045ed7136b7 100644 --- a/lib/system/cyclebreaker.nim +++ b/lib/system/cyclebreaker.nim @@ -53,17 +53,70 @@ depth-first traversal suffices. ]# +when not declared(benign): + {.pragma: benign.} +when not declared(compilerRtl): + {.pragma: compilerRtl.} +when not declared(inl): + {.pragma: inl.} + +when not declared(traceCollector): + const traceCollector = defined(nimTraceCollector) + +when traceCollector: + proc cprintf(fmt: cstring) {.importc: "printf", header: "", varargs, discardable.} + +when not declared(head): + type RefHeader* = object + rc: int + template head(p: pointer): ptr RefHeader = + cast[ptr RefHeader](cast[int](p) -% sizeof(RefHeader)) + +when not declared(Cell): + type Cell = ptr RefHeader + +template payloadPtr(c: Cell): pointer = + cast[pointer](cast[int](c) +% sizeof(RefHeader)) + +when not declared(rcIncrement): + when defined(gcOrc): + const + rcIncrement* = 0b10000 + rcMask* = 0b1111 + rcShift* = 4 + else: + const + rcIncrement* = 0b1000 + rcMask* = 0b111 + rcShift* = 3 + +when not declared(maybeCycleFlag): + when declared(maybeCycle): + const maybeCycleFlag = maybeCycle + elif defined(gcOrc): + const maybeCycleFlag = 0b100 + else: + const maybeCycleFlag = 0 + +when not declared(nimRawDispose): + proc nimRawDispose(p: pointer, alignment: int) {.importc: "nimRawDispose", raises: [].} + include cellseqs_v2 +proc releaseGraph(p: pointer; desc: PNimTypeV2) {.gcsafe, raises: [].} + const colGreen = 0b000 colYellow = 0b001 colRed = 0b010 colorMask = 0b011 -type - TraceProc = proc (p, env: pointer) {.nimcall, benign, raises: [].} - DisposeProc = proc (p: pointer) {.nimcall, benign, raises: [].} +when not declared(TraceProc): + type TraceProc = proc (p, env: pointer) {.nimcall, benign, gcsafe, raises: [].} +when not declared(DisposeProc): + type DisposeProc = proc (p: pointer) {.nimcall, benign, gcsafe, raises: [].} +when not declared(DestructorProc): + type DestructorProc = proc (p: pointer) {.nimcall, benign, gcsafe, raises: [].} template color(c): untyped = c.rc and colorMask template setColor(c, col) = @@ -79,14 +132,14 @@ type GcEnv = object traceStack: CellSeq[ptr pointer] -proc trace(p: pointer; desc: PNimTypeV2; j: var GcEnv) {.inline.} = +proc trace(p: pointer; desc: PNimTypeV2; j: var GcEnv) {.inline, gcsafe, raises: [].} = when false: cprintf("[Trace] desc: %p %p\n", desc, p) cprintf("[Trace] trace: %p\n", desc.traceImpl) if desc.traceImpl != nil: cast[TraceProc](desc.traceImpl)(p, addr(j)) -proc nimTraceRef(q: pointer; desc: PNimTypeV2; env: pointer) {.compilerRtl.} = +proc nimTraceRefImpl*(q: pointer; desc: PNimTypeV2; env: pointer) {.compilerRtl, gcsafe.} = let p = cast[ptr pointer](q) when traceCollector: cprintf("[Trace] raw: %p\n", p) @@ -95,7 +148,7 @@ proc nimTraceRef(q: pointer; desc: PNimTypeV2; env: pointer) {.compilerRtl.} = var j = cast[ptr GcEnv](env) j.traceStack.add(p, desc) -proc nimTraceRefDyn(q: pointer; env: pointer) {.compilerRtl.} = +proc nimTraceRefDynImpl*(q: pointer; env: pointer) {.compilerRtl, gcsafe.} = let p = cast[ptr pointer](q) when traceCollector: cprintf("[TraceDyn] raw: %p\n", p) @@ -104,20 +157,35 @@ proc nimTraceRefDyn(q: pointer; env: pointer) {.compilerRtl.} = var j = cast[ptr GcEnv](env) j.traceStack.add(p, cast[ptr PNimTypeV2](p[])[]) +when not declared(nimTraceClosure): + proc nimTraceClosureImpl*(p, env: pointer) {.compilerRtl, nimcall, benign, gcsafe, raises: [].} = + let slot = cast[ptr pointer](cast[int](p) +% sizeof(pointer)) + nimTraceRefImpl(slot, cast[ptr PNimTypeV2](slot[])[], env) + +when not declared(nimTraceRef): + proc nimTraceRef(q: pointer; desc: PNimTypeV2; env: pointer) {.compilerRtl, inline, gcsafe.} = + nimTraceRefImpl(q, desc, env) + +when not declared(nimTraceRefDyn): + proc nimTraceRefDyn(q: pointer; env: pointer) {.compilerRtl, inline, gcsafe.} = + nimTraceRefDynImpl(q, env) + var markerGeneration: int -proc breakCycles(s: Cell; desc: PNimTypeV2) = +proc breakCycles(s: Cell; desc: PNimTypeV2) {.gcsafe, raises: [].} = let markerColor = if (markerGeneration and 1) == 0: colRed else: colYellow - atomicInc markerGeneration + inc markerGeneration when traceCollector: - cprintf("[BreakCycles] starting: %p %s RC %ld trace proc %p\n", - s, desc.name, s.rc shr rcShift, desc.traceImpl) + cprintf("[BreakCycles] starting: %p desc=%p RC %ld trace proc %p\n", + s, desc, s.rc shr rcShift, desc.traceImpl) var j: GcEnv init j.traceStack s.setColor markerColor - trace(s +! sizeof(RefHeader), desc, j) + when maybeCycleFlag != 0: + s.rc = s.rc and not maybeCycleFlag + trace(payloadPtr(s), desc, j) while j.traceStack.len > 0: let (u, desc) = j.traceStack.pop() @@ -125,6 +193,8 @@ proc breakCycles(s: Cell; desc: PNimTypeV2) = let t = head(p) if t.color != markerColor: t.setColor markerColor + when maybeCycleFlag != 0: + t.rc = t.rc and not maybeCycleFlag trace(p, desc, j) when traceCollector: cprintf("[BreakCycles] followed: %p RC %ld\n", t, t.rc shr rcShift) @@ -139,10 +209,10 @@ proc breakCycles(s: Cell; desc: PNimTypeV2) = # anyhow as a link that the produced destructor does not have to follow: u[] = nil when traceCollector: - cprintf("[Bug] %p %s RC %ld\n", t, desc.name, t.rc shr rcShift) + cprintf("[Bug] %p desc=%p RC %ld\n", t, desc, t.rc shr rcShift) deinit j.traceStack -proc thinout*[T](x: ref T) {.inline.} = +proc thinout*[T](x: ref T) {.inline, gcsafe, raises: [].} = ## turn the subgraph starting with `x` into its spanning tree by ## `nil`'ing out any pointers that would harm the spanning tree ## structure. Any back pointers that introduced cycles @@ -151,16 +221,32 @@ proc thinout*[T](x: ref T) {.inline.} = ## and its associated cost model. proc getDynamicTypeInfo[T](x: T): PNimTypeV2 {.magic: "GetTypeInfoV2", noSideEffect.} - breakCycles(head(cast[pointer](x)), getDynamicTypeInfo(x[])) - -proc thinout*[T: proc](x: T) {.inline.} = + var desc: PNimTypeV2 + when declared(getDynamicTypeInfo): + desc = getDynamicTypeInfo(x[]) + let dynDesc = cast[ptr PNimTypeV2](cast[pointer](x))[] + if desc == nil: + desc = dynDesc + let ti = desc + when defined(debugThinout): + echo "[thinout ref] ptr=", cast[int](x), " desc=", cast[int](ti), " traceImpl=", cast[int](ti.traceImpl) + releaseGraph(cast[pointer](x), ti) + when not defined(gcOrc) or defined(nimThinout): + breakCycles(head(cast[pointer](x)), ti) + +proc thinout*[T: proc](x: T) {.inline, gcsafe, raises: [].} = proc rawEnv[T: proc](x: T): pointer {.noSideEffect, inline.} = {.emit: """ `result` = `x`.ClE_0; """.} let p = rawEnv(x) - breakCycles(head(p), cast[ptr PNimTypeV2](p)[]) + let desc = cast[ptr PNimTypeV2](p)[] + when defined(debugThinout): + echo "[thinout] env=", cast[int](p), " typeInfo=", cast[int](desc), " traceImpl=", cast[int](desc.traceImpl) + releaseGraph(p, desc) + when not defined(gcOrc) or defined(nimThinout): + breakCycles(head(p), desc) proc nimDecRefIsLastCyclicDyn(p: pointer): bool {.compilerRtl, inl.} = if p != nil: @@ -182,3 +268,25 @@ proc nimDecRefIsLastCyclicStatic(p: pointer; desc: PNimTypeV2): bool {.compilerR else: dec cell.rc, rcIncrement #cprintf("[DeCREF] %p %s %ld\n", p, desc.name, cell.rc) + +proc releaseGraph(p: pointer; desc: PNimTypeV2) {.gcsafe, raises: [].} = + if desc == nil or desc.traceImpl == nil: return + var env: GcEnv + init env.traceStack + trace(p, desc, env) + while env.traceStack.len > 0: + let (slot, childDesc) = env.traceStack.pop() + let child = slot[] + if child != nil: + slot[] = nil + releaseGraph(child, childDesc) + let cell = head(child) + when maybeCycleFlag != 0: + cell.rc = cell.rc and not maybeCycleFlag + let isLast = nimDecRefIsLastCyclicStatic(child, childDesc) + if isLast: + if childDesc.destructor != nil: + cast[DestructorProc](childDesc.destructor)(child) + let disposer = cast[proc (p: pointer, alignment: int) {.nimcall, gcsafe, raises: [].}](nimRawDispose) + disposer(child, childDesc.align) + deinit env.traceStack diff --git a/lib/system/orc.nim b/lib/system/orc.nim index cb84a9ade1dcb..132401c838357 100644 --- a/lib/system/orc.nim +++ b/lib/system/orc.nim @@ -140,21 +140,72 @@ proc nimTraceRefDyn(q: pointer; env: pointer) {.compilerRtl, inl.} = var j = cast[ptr GcEnv](env) j.traceStack.add(p, cast[ptr PNimTypeV2](p[])[]) +when not declared(nimTraceClosure): + proc nimTraceClosure(p, env: pointer) {.compilerRtl, nimcall, benign, gcsafe, raises: [].} = + let slot = cast[ptr pointer](cast[int](p) +% sizeof(pointer)) + nimTraceRef(slot, cast[ptr PNimTypeV2](slot[])[], env) + var roots {.threadvar.}: CellSeq[Cell] +proc compactRoots(s: var CellSeq[Cell]) = + ## Compact the roots set by dropping detached entries and fixing rootIdx. + if s.len == 0 or s.d == nil: + return + var dst = 0 + for src in 0 ..< s.len: + let cell = s.d[src][0] + if cell != nil and cell.rootIdx > 0: + if dst != src: + s.d[dst] = s.d[src] + s.d[dst][0].rootIdx = dst +% 1 + dst = dst +% 1 + else: + if cell != nil: + cell.rootIdx = 0 + if dst < s.len: + s.len = dst + proc unregisterCycle(s: Cell) = # swap with the last element. O(1) - let - rootIdx = s.rootIdx - idx = rootIdx -% 1 + if s.rootIdx <= 0: + s.rootIdx = 0 + compactRoots(roots) + return + if roots.len <= 0: + s.rootIdx = 0 + return + var + idx = s.rootIdx -% 1 last = roots.len -% 1 - when false: - if idx >= roots.len or idx < 0: - cprintf("[Bug!] %ld %ld\n", idx, roots.len) - rawQuit 1 - roots.d[idx] = roots.d[last] - roots.d[idx][0].rootIdx = rootIdx + template ensureValidIndex(): bool = + (idx >= 0) and (idx <= last) and roots.d[idx][0] == s + if not ensureValidIndex(): + compactRoots(roots) + if s.rootIdx <= 0 or roots.len <= 0: + s.rootIdx = 0 + return + idx = s.rootIdx -% 1 + last = roots.len -% 1 + if not ensureValidIndex(): + var found = false + for i in countdown(last, 0): + if roots.d[i][0] == s: + idx = i + s.rootIdx = i +% 1 + last = roots.len -% 1 + found = true + break + if not found or not ensureValidIndex(): + s.rootIdx = 0 + compactRoots(roots) + return + let moved = roots.d[last] + roots.d[idx] = moved + if idx != last: + let movedCell = moved[0] + if movedCell != nil: + movedCell.rootIdx = idx +% 1 roots.len = last s.rootIdx = 0 diff --git a/tests/arc/tasync_asynccheck_server.nim b/tests/arc/tasync_asynccheck_server.nim new file mode 100644 index 0000000000000..2fd3b454f9bc0 --- /dev/null +++ b/tests/arc/tasync_asynccheck_server.nim @@ -0,0 +1,93 @@ +discard """ + cmd: '''nim c --mm:orc $file''' +""" + +import std/[asyncdispatch, asyncnet, strutils] +from stdtest/netutils import bindAvailablePort + +when defined(gcArc) or defined(gcOrc): + const + mmName = when defined(gcArc): "arc" else: "orc" + swarmSize = 3 + messagesPerClient = 4 + maxPollSpins = 256 + + type + TrackerObj = object + id: int + + var + destroyedCount = 0 + nextId = 0 + completedClients = 0 + + proc `=destroy`(x: var TrackerObj) = + inc destroyedCount + + proc newTracker(): ref TrackerObj = + inc nextId + new result + result.id = nextId + + proc handleClient(client: AsyncSocket; tracker: ref TrackerObj) {.async.} = + for i in 0.. 0, + mmName & " server should receive non-empty payload" + await client.send("done\n") + client.close() + inc completedClients + + proc acceptLoop(server: AsyncSocket) {.async.} = + while completedClients < swarmSize: + try: + let client = await server.accept() + let tracker = newTracker() + asyncCheck handleClient(client, tracker) + except OSError: + break + + proc runClient(port: Port; tracker: ref TrackerObj) {.async.} = + let sock = await asyncnet.dial("127.0.0.1", port) + for i in 0.. 0 + doAssert not tracker.isNil, + mmName & " tracker should stay alive before await" + await sleepAsync(0) + doAssert not tracker.isNil, + mmName & " tracker should stay alive until async step finishes" + finished = true + + var fut = worker() + asyncCheck fut + + let spins = spinUntil(proc (): bool = finished, maxSpins) + doAssert finished, + mmName & " asyncCheck future should finish within polling budget" + doAssert spins < maxSpins, + mmName & " asyncCheck future waited too long to finish" + + discard spinUntil(proc (): bool = destroyedCount >= expectedDestroyed, maxSpins) + + fut = nil + tracker = nil + + discard spinUntil(proc (): bool = destroyedCount >= expectedDestroyed, maxSpins) + + doAssert destroyedCount == expectedDestroyed, + mmName & " asyncCheck should release captured refs in same turn" + + proc ensureFutureCallbackBreaksCycle() = + let expectedDestroyed = destroyedCount + 1 + var callbackRan = false + block: + var tracker = newTracker() + let expectedId = tracker.id + var fut = newFuture[int]("async closure cycle") + doAssert not fut.isNil, + mmName & " future allocation failed unexpectedly" + let capturedFuture = fut + + fut.addCallback(proc () = + doAssert not capturedFuture.isNil + doAssert capturedFuture.read() == expectedId + doAssert tracker.id == expectedId + callbackRan = true + ) + + fut.complete(expectedId) + + let spins = spinUntil(proc (): bool = callbackRan, maxSpins) + doAssert callbackRan, + mmName & " future callback should run within polling budget" + doAssert spins < maxSpins, + mmName & " future callback took too long to run" + + discard spinUntil(proc (): bool = destroyedCount >= expectedDestroyed, maxSpins) + + fut = nil + tracker = nil + + discard spinUntil(proc (): bool = destroyedCount >= expectedDestroyed, maxSpins) + + doAssert destroyedCount == expectedDestroyed, + mmName & " future callback cycle should be broken deterministically" + + const runs = 3 + + destroyedCount = 0 + nextId = 0 + for _ in 0..