diff --git a/logos_delivery/channels/api/channel_lifecycle.nim b/logos_delivery/channels/api/channel_lifecycle.nim index 6b233faca..e0c4adb70 100644 --- a/logos_delivery/channels/api/channel_lifecycle.nim +++ b/logos_delivery/channels/api/channel_lifecycle.nim @@ -16,10 +16,11 @@ const SdsJobId = "sds" ## One persistency job shared by every channel's SDS state; rows are ## keyed by channelId. -proc sdsPersistence(): Opt[Persistence] = - ## SDS backend from the Persistency singleton; memory-only fallback when - ## it is unavailable (e.g. unit tests). - let p = Persistency.instance().valueOr: +proc sdsPersistence(brokerCtx: BrokerContext): Opt[Persistence] = + ## SDS backend from the node's Persistency instance, resolved via the + ## context-scoped GetPersistency broker; memory-only fallback when no + ## provider is installed (e.g. unit tests). + let p = GetPersistency.request(brokerCtx).valueOr: info "SDS persistence disabled, running memory-only", reason = $error return Opt.none(Persistence) let job = p.openJob(SdsJobId).valueOr: @@ -60,7 +61,7 @@ proc createReliableChannel*( cc.sdsAcknowledgementTimeoutMs.get(DefaultAcknowledgementTimeoutMs), maxRetransmissions: cc.sdsMaxRetransmissions.get(DefaultMaxRetransmissions), causalHistorySize: cc.sdsCausalHistorySize.get(DefaultCausalHistorySize), - persistence: sdsPersistence(), + persistence: sdsPersistence(self.brokerCtx), ) let chn = ReliableChannel.new( diff --git a/logos_delivery/waku/persistency/persistency.nim b/logos_delivery/waku/persistency/persistency.nim index 46ce7622e..1d4f095be 100644 --- a/logos_delivery/waku/persistency/persistency.nim +++ b/logos_delivery/waku/persistency/persistency.nim @@ -10,7 +10,7 @@ ## it. Cheapest, no map lookup per call: ## ## ```nim -## let p = Persistency.instance("/var/lib/wakustore").get() +## let p = Persistency.new("/var/lib/wakustore").get() ## let j = p.openJob("alpha").get() ## await j.persistPut("msg", k, payload) ## let v = await j.get("msg", k) @@ -40,7 +40,7 @@ {.push raises: [].} -import std/[locks, os, sequtils, tables] +import std/[os, sequtils, tables] import chronos, chronicles, results import brokers/[event_broker, request_broker, broker_context] import ./[types, keys, payload, backend_comm, backend_thread] @@ -52,6 +52,11 @@ logScope: const DefaultStoragePath* = "./data" +const InMemoryStoragePath* = ":memory:" + ## Pass as ``rootDir`` to keep every job in a private in-memory SQLite + ## database (one per job worker; nothing touches the filesystem). + ## State is lost when the job closes. Intended for tests. + # ── Driver types ──────────────────────────────────────────────────────── type @@ -67,32 +72,36 @@ type Persistency* = ref object ## Per-root coordinator. One Persistency instance manages a directory ## of per-job SQLite files at ``rootDir/.db``. + ## + ## ``GetPersistency`` RequestBroker will give access to the active + ## instance of the current node. + ## Owner must close it properly (``Persistency.close``) to stop all jobs and + ## free the threads. rootDir*: string jobs*: Table[string, Job] -# ── Singleton state ───────────────────────────────────────────────────── +# ── Instance access broker ────────────────────────────────────────────── # -# Persistency is a process-wide singleton: one rootDir at a time. The -# `instance` factory is the only public constructor; `new` below is -# private and skips the singleton bookkeeping (used internally and never -# called twice with conflicting rootDirs). +# The owner of a Persistency instance (the Waku node) provides it under +# its BrokerContext; consumers on the same context (e.g. the SDS channel +# layer) resolve it without holding a reference to the owner. Sync and +# same-thread: the provider returns the ref directly, no copies or +# marshalling. One instance per owner — nothing is process-global. -var - gPersistency {.global.}: Persistency - gPersistencyLock {.global.}: Lock - -once: - gPersistencyLock.initLock() +RequestBroker(sync): + proc getPersistency(): Result[Persistency, string] # ── Lifecycle ─────────────────────────────────────────────────────────── proc dbPathFor(p: Persistency, jobId: string): string = + if p.rootDir == InMemoryStoragePath: + return InMemoryStoragePath p.rootDir / (jobId & ".db") -proc new(T: type Persistency, rootDir: string): Result[T, PersistencyError] = - ## Private. Build a Persistency value without touching the singleton - ## slot. Validates ``rootDir`` but does **not** create it — directory - ## materialisation is deferred to the first ``openJob`` call. Semantics: +proc new*(T: type Persistency, rootDir: string): Result[T, PersistencyError] = + ## Build a Persistency instance for ``rootDir``. Validates ``rootDir`` + ## but does **not** create it — directory materialisation is deferred + ## to the first ``openJob`` call. Semantics: ## ## * If ``rootDir`` is empty, returns ``peInvalidArgument``. ## * If ``rootDir`` exists and is a directory, accept it. @@ -102,6 +111,8 @@ proc new(T: type Persistency, rootDir: string): Result[T, PersistencyError] = ## existing ancestor must be a directory; otherwise returns ## ``peInvalidArgument``. This catches "obviously broken" paths early ## without actually touching the filesystem. + if rootDir == InMemoryStoragePath: + return ok(T(rootDir: rootDir, jobs: initTable[string, Job]())) if rootDir.len == 0: return err(persistencyErr(peInvalidArgument, "rootDir is empty")) if fileExists(rootDir) and not dirExists(rootDir): @@ -126,6 +137,8 @@ proc new(T: type Persistency, rootDir: string): Result[T, PersistencyError] = proc ensureRootDir(p: Persistency): Result[void, PersistencyError] = ## Materialise ``rootDir`` on demand. Idempotent; called from ## ``openJob`` so an unused Persistency leaves no directory behind. + if p.rootDir == InMemoryStoragePath: + return ok() if dirExists(p.rootDir): return ok() try: @@ -135,65 +148,6 @@ proc ensureRootDir(p: Persistency): Result[void, PersistencyError] = err(persistencyErr(peBackend, "createDir failed: " & getCurrentExceptionMsg())) return ok() -proc reset*(T: type Persistency) {.gcsafe.} = - ## Tear down the singleton: close every open job, clear the Teardown - ## provider, and free the slot so a subsequent ``Persistency.instance`` - ## starts fresh. Idempotent. Tests use this in `defer`;. - {.cast(gcsafe).}: - acquire(gPersistencyLock) - defer: - release(gPersistencyLock) - if gPersistency != nil: - let p = gPersistency - gPersistency = nil - p.close() - -proc instance*( - T: type Persistency, rootDir: string -): Result[T, PersistencyError] {.gcsafe.} = - ## Get-or-init the process-wide Persistency singleton. - ## - ## * First call: validates ``rootDir`` (without creating it) and - ## registers the Teardown handler. The directory itself is created - ## lazily by the first ``openJob`` call, so a Persistency that never - ## opens a job leaves no filesystem footprint. - ## * Later calls with the same ``rootDir``: returns the live instance - ## (idempotent). - ## * Later calls with a different ``rootDir``: returns - ## ``peInvalidArgument`` — the singleton can only be re-targeted via - ## ``Persistency.reset`` (or by the Teardown shutdown flow). - {.cast(gcsafe).}: - acquire(gPersistencyLock) - defer: - release(gPersistencyLock) - - if gPersistency != nil: - if gPersistency.rootDir == rootDir: - return ok(gPersistency) - return err( - persistencyErr( - peInvalidArgument, - "Persistency already initialised with rootDir " & gPersistency.rootDir & - "; cannot re-init with " & rootDir, - ) - ) - - let p = ?Persistency.new(rootDir) - gPersistency = p - return ok(p) - -proc instance*(T: type Persistency): Result[T, PersistencyError] {.gcsafe.} = - ## No-args form: succeeds only if the singleton is already initialised. - ## Use this from services that must not be the first to touch - ## persistency. - {.cast(gcsafe).}: - acquire(gPersistencyLock) - defer: - release(gPersistencyLock) - if gPersistency.isNil: - return err(persistencyErr(peClosed, "Persistency not initialised")) - return ok(gPersistency) - proc openJob*(p: Persistency, jobId: string): Result[Job, PersistencyError] = ## Open-or-create a job under this Persistency. ## @@ -240,6 +194,8 @@ proc dropJob*(p: Persistency, jobId: string) = ## Close the job if open, then delete its DB file (plus -wal / -shm ## sidecars). Best-effort: a missing file is not an error. p.closeJob(jobId) + if p.rootDir == InMemoryStoragePath: + return let path = dbPathFor(p, jobId) for suffix in ["", "-wal", "-shm"]: try: diff --git a/logos_delivery/waku/waku.nim b/logos_delivery/waku/waku.nim index 89af02454..fec97b9ef 100644 --- a/logos_delivery/waku/waku.nim +++ b/logos_delivery/waku/waku.nim @@ -88,6 +88,8 @@ type Waku* = ref object ## Implements `KernelApi` (ops in `waku/api/*`). brokerCtx*: BrokerContext + persistency*: Persistency + proc setupSwitchServices( waku: Waku, conf: WakuConf, circuitRelay: Relay, rng: crypto.Rng ) = @@ -363,6 +365,14 @@ proc startDnsDiscoveryRetryLoop(waku: Waku): Future[void] {.async.} = error "failed to connect to dynamic bootstrap nodes: " & getCurrentExceptionMsg() return +proc closePersistency(waku: Waku) = + ## Clear the GetPersistency provider and close the instance (joins any + ## job worker threads). Idempotent; shared by `stop` and a failed `start`. + GetPersistency.clearProvider(waku.brokerCtx) + if not waku.persistency.isNil(): + waku.persistency.close() + waku.persistency = nil + proc start*(waku: Waku): Future[Result[void, string]] {.async: (raises: []).} = if waku.node.started: warn "start: waku node already started" @@ -371,6 +381,21 @@ proc start*(waku: Waku): Future[Result[void, string]] {.async: (raises: []).} = info "Retrieve dynamic bootstrap nodes" let conf = waku.conf + ## Create this node's Persistency instance and provide it under the node's + ## BrokerContext first, so any later startup stage can restore persisted + ## state through it. Inert until the first openJob. The defer below tears + ## it down again on every one of start's error return paths. + waku.persistency = Persistency.new(conf.localStoragePath).valueOr: + error "Failed to initialize persistency instance", error = $error + return err("Failed to initialize persistency instance: " & $error) + discard GetPersistency.reprovideIt(waku.brokerCtx): + ok(waku.persistency) + + var startSucceeded = false + defer: + if not startSucceeded: + waku.closePersistency() + if conf.dnsDiscoveryConf.isSome(): let dnsDiscoveryConf = waku.conf.dnsDiscoveryConf.get() let dynamicBootstrapNodesRes = @@ -391,12 +416,6 @@ proc start*(waku: Waku): Future[Result[void, string]] {.async: (raises: []).} = else: waku.dynamicBootstrapNodes = dynamicBootstrapNodesRes.get() - ## Initialize persistency singleton instance - we don't need the instance itself here, - ## but this ensures it's initialized before any store job starts. - discard Persistency.instance(conf.localStoragePath).valueOr: - error "Failed to initialize persistency instance", error = $error - return err("Failed to initialize persistency instance: " & $error) - (await startNode(waku.node, waku.conf, waku.dynamicBootstrapNodes)).isOkOr: return err("error while calling startNode: " & $error) @@ -512,6 +531,7 @@ proc start*(waku: Waku): Future[Result[void, string]] {.async: (raises: []).} = ) waku.healthMonitor.setOverallHealth(HealthStatus.READY) + startSucceeded = true return ok() proc stop*(waku: Waku): Future[Result[void, string]] {.async: (raises: []).} = @@ -521,7 +541,7 @@ proc stop*(waku: Waku): Future[Result[void, string]] {.async: (raises: []).} = try: waku.healthMonitor.setOverallHealth(HealthStatus.SHUTTING_DOWN) - Persistency.reset() + waku.closePersistency() if not waku.metricsServer.isNil(): await waku.metricsServer.stop() diff --git a/tests/channels/test_reliable_channel_send_receive.nim b/tests/channels/test_reliable_channel_send_receive.nim index 41ce46327..7203f8188 100644 --- a/tests/channels/test_reliable_channel_send_receive.nim +++ b/tests/channels/test_reliable_channel_send_receive.nim @@ -585,12 +585,11 @@ suite "Reliable Channel - SDS persistence": channelId = ChannelId("sds-persist-channel") contentTopic = ContentTopic("/reliable-channel/test/persist") - Persistency.reset() let root = getTempDir() / ("reliable_channel_sds_" & $epochTime().int) removeDir(root) - let persistency = Persistency.instance(root).expect("persistency init") + let persistency = Persistency.new(root).expect("persistency init") defer: - Persistency.reset() + persistency.close() removeDir(root) var waku: LogosDelivery @@ -599,6 +598,9 @@ suite "Reliable Channel - SDS persistence": waku = (await LogosDelivery.new(createApiNodeConf())).expect("LogosDelivery.new") manager = waku.reliableChannelManager + discard GetPersistency.reprovideIt(manager.brokerCtx): + ok(persistency) + setNoopEncryption() MessagingSend.replaceProvider( @@ -841,12 +843,11 @@ suite "Reliable Channel - SDS lifecycle": contentTopic = ContentTopic("/reliable-channel/test/restore") let appPayload = "survive restart".toBytes() - Persistency.reset() let root = getTempDir() / ("reliable_channel_sds_restore_" & $epochTime().int) removeDir(root) - let persistency = Persistency.instance(root).expect("persistency init") + let persistency = Persistency.new(root).expect("persistency init") defer: - Persistency.reset() + persistency.close() removeDir(root) var waku: LogosDelivery @@ -857,6 +858,9 @@ suite "Reliable Channel - SDS lifecycle": waku = (await LogosDelivery.new(createApiNodeConf())).expect("LogosDelivery.new") manager = waku.reliableChannelManager + discard GetPersistency.reprovideIt(manager.brokerCtx): + ok(persistency) + setNoopEncryption() discard manager @@ -991,12 +995,11 @@ suite "Reliable Channel - SDS protocol semantics": channelId = ChannelId("sds-ack-channel") contentTopic = ContentTopic("/reliable-channel/test/ack") - Persistency.reset() let root = getTempDir() / ("reliable_channel_sds_ack_" & $epochTime().int) removeDir(root) - let persistency = Persistency.instance(root).expect("persistency init") + let persistency = Persistency.new(root).expect("persistency init") defer: - Persistency.reset() + persistency.close() removeDir(root) var waku: LogosDelivery @@ -1007,6 +1010,9 @@ suite "Reliable Channel - SDS protocol semantics": waku = (await LogosDelivery.new(createApiNodeConf())).expect("LogosDelivery.new") manager = waku.reliableChannelManager + discard GetPersistency.reprovideIt(manager.brokerCtx): + ok(persistency) + setNoopEncryption() var capturedWires: seq[seq[byte]] diff --git a/tests/ffi/test_ffi_persistency_lifecycle.nim b/tests/ffi/test_ffi_persistency_lifecycle.nim new file mode 100644 index 000000000..99528c2f1 --- /dev/null +++ b/tests/ffi/test_ffi_persistency_lifecycle.nim @@ -0,0 +1,389 @@ +{.used.} + +## FFI-level regression guard for the former persistency singleton's +## thread/lifetime mismatch, driven through the real C ABI: the driver +## `dlopen`s `liblogosdelivery` and calls the exported `logosdelivery_*` +## entry points, exactly as a module host does. +## +## Historically `Persistency` was a process-global (`gPersistency`) whose +## memory belonged to the FFI thread that first initialised it; with two +## contexts alive the second one adopted the first one's instance and read +## a foreign heap. Persistency is now owned per node (`Waku.persistency`, +## resolved via the context-scoped `GetPersistency` broker), so every case +## below must pass: +## +## case stop-steals-persistence +## ctx1 stop_node closes only ctx1's own persistency; ctx2 must keep +## its SDS persistence working. +## +## case destroy-without-stop +## destroying ctx1 without stop must not corrupt ctx2's persistency. +## Historically a UB probe (stale global into a released heap -> +## SIGSEGV on Linux, zombie singleton on macOS/arm64); kept as a guard. +## The destroy-without-stop teardown gap itself is tracked separately +## (issue #4108). +## +## case different-storage-paths +## two contexts with different local-storage-paths must both start; +## the former singleton refused the second rootDir. +## +## Each case runs in a child process (the second one can fault) with its +## output captured to a file, so a crash is an exit code rather than a dead +## test binary. +## +## Requires the shared library. Build it with: +## nim c $(tr '\n' ' ' < nimble.paths) --nimMainPrefix:liblogosdelivery \ +## --out:build/liblogosdelivery.dylib --app:lib --noMain --threads:on \ +## --mm:refc --opt:speed --passL:librln_v2.0.2.a --passL:-lm \ +## -d:chronicles_log_level=INFO library/liblogosdelivery.nim +## On Linux add `--passL:-Wl,-Bsymbolic` (as the repo's own Linux target +## does) so the driver's Nim runtime does not interpose the library's. +## Override the location with LIBLOGOSDELIVERY=. +## +## Not registered in an aggregate -- it needs the shared library built +## first (see above). Run: +## make test tests/ffi/test_ffi_persistency_lifecycle.nim + +import std/[atomics, dynlib, json, os, osproc, strutils] +import testutils/unittests + +const + RetOk = 0 + + CaseStopSteals = "--case-stop-steals-persistence" + CaseDestroyOnly = "--case-destroy-without-stop" + CaseTwoPaths = "--case-different-storage-paths" + + DisabledMarker = "SDS persistence disabled" + ## Logged by `sdsPersistence()` when the singleton is unusable. + + ContentTopic = "/lm-repro/1/channel/proto" + SenderId = "repro-sender" + + LibSuffix = + when defined(macosx): + ".dylib" + elif defined(windows): + ".dll" + else: + ".so" + +# ── C ABI surface ─────────────────────────────────────────────────────── + +type + FfiCallback = proc(callerRet: cint, msg: ptr cchar, len: csize_t, userData: pointer) {. + cdecl, gcsafe, raises: [] + .} + + CreateNodeFn = proc(configJson: cstring, cb: FfiCallback, userData: pointer): pointer {. + cdecl, gcsafe + .} + CtxFn = proc(ctx: pointer, cb: FfiCallback, userData: pointer): cint {.cdecl, gcsafe.} + ChannelCreateFn = proc( + ctx: pointer, + cb: FfiCallback, + userData: pointer, + channelId, contentTopic, senderId: cstring, + ): cint {.cdecl, gcsafe.} + ChannelExistsFn = proc( + ctx: pointer, cb: FfiCallback, userData: pointer, channelId: cstring + ): cint {.cdecl, gcsafe.} + + Api = object + createNode: CreateNodeFn + startNode: CtxFn + stopNode: CtxFn + destroy: CtxFn + channelCreate: ChannelCreateFn + channelExists: ChannelExistsFn + + Slot = object + ## Shared-memory callback landing pad. The callback fires on the + ## library's FFI thread, so nothing GC'd may cross it. + done: Atomic[int] + ret: Atomic[int] + len: Atomic[int] + buf: array[2048, char] + +proc libPath(): string = + let fromEnv = getEnv("LIBLOGOSDELIVERY") + if fromEnv.len > 0: + return fromEnv + return getCurrentDir() / "build" / ("liblogosdelivery" & LibSuffix) + +proc onDone( + callerRet: cint, msg: ptr cchar, len: csize_t, userData: pointer +) {.cdecl, gcsafe, raises: [].} = + let s = cast[ptr Slot](userData) + if s.isNil(): + return + var n = int(len) + if n > s.buf.len - 1: + n = s.buf.len - 1 + if n > 0 and not msg.isNil(): + copyMem(addr s.buf[0], msg, n) + s.buf[n] = '\0' + s.len.store(n) + s.ret.store(int(callerRet)) + s.done.store(1) + +proc armSlot(s: ptr Slot) = + s.done.store(0) + s.ret.store(-1) + s.len.store(0) + +proc awaitSlot( + s: ptr Slot, timeoutMs = 60_000 +): tuple[ok: bool, ret: int, msg: string] = + ## `msg` is returned raw. It is CBOR, not a bare string: a channel id + ## "before" arrives as 0x66 'b' 'e' 'f' 'o' 'r' 'e' (major type 3, len 6), + ## which prints as "fbefore". Only `ret` is used for assertions here. + var waited = 0 + while s.done.load() == 0 and waited < timeoutMs: + sleep(10) + waited += 10 + if s.done.load() == 0: + return (false, -1, "timeout after " & $timeoutMs & "ms") + let n = s.len.load() + var m = newString(n) + if n > 0: + copyMem(addr m[0], addr s.buf[0], n) + return (true, s.ret.load(), m) + +proc need(lib: LibHandle, name: string): pointer = + let p = lib.symAddr(name) + if p.isNil(): + quit("missing symbol " & name & " in " & libPath(), 2) + return p + +proc loadApi(): Api = + let lib = loadLib(libPath()) + if lib.isNil(): + quit("cannot load " & libPath(), 2) + + Api( + createNode: cast[CreateNodeFn](lib.need("logosdelivery_create_node")), + startNode: cast[CtxFn](lib.need("logosdelivery_start_node")), + stopNode: cast[CtxFn](lib.need("logosdelivery_stop_node")), + destroy: cast[CtxFn](lib.need("logosdelivery_destroy")), + channelCreate: cast[ChannelCreateFn](lib.need("logosdelivery_channel_create")), + channelExists: cast[ChannelExistsFn](lib.need("logosdelivery_channel_exists")), + ) + +# ── driver helpers ────────────────────────────────────────────────────── + +var failed = false + +proc note(step: string, r: tuple[ok: bool, ret: int, msg: string]) = + echo " [", step, "] ok=", r.ok, " ret=", r.ret, " msg=", r.msg + +proc expectOk(step: string, r: tuple[ok: bool, ret: int, msg: string]) = + note(step, r) + if not r.ok or r.ret != RetOk: + echo " FAIL: ", step, " expected RET_OK" + failed = true + +proc nodeConfig(storagePath: string, tcpPort, discv5Port: int): string = + $( + %*{ + "mode": "Core", + "preset": "logos.dev", + "messagingOverrides": { + "log-level": "INFO", + "local-storage-path": storagePath, + "tcp-port": $tcpPort, + "discv5-udp-port": $discv5Port, + }, + } + ) + +proc createCtx( + api: Api, s: ptr Slot, label, storagePath: string, tcpPort, discv5Port: int +): pointer = + armSlot(s) + let ctx = + api.createNode(nodeConfig(storagePath, tcpPort, discv5Port).cstring, onDone, s) + if ctx.isNil(): + echo " FAIL: ", label, " create_node returned nil" + failed = true + return nil + expectOk(label & " create_node", awaitSlot(s)) + return ctx + +proc call(api: Api, s: ptr Slot, label: string, fn: CtxFn, ctx: pointer) = + armSlot(s) + discard fn(ctx, onDone, s) + expectOk(label, awaitSlot(s)) + +proc createChannel(api: Api, s: ptr Slot, label: string, ctx: pointer, id: string) = + armSlot(s) + discard api.channelCreate( + ctx, onDone, s, id.cstring, ContentTopic.cstring, SenderId.cstring + ) + expectOk(label, awaitSlot(s)) + +proc churn(api: Api, s: ptr Slot, ctx: pointer, rounds = 500) = + ## Allocate and release on the *target context's* FFI thread without + ## touching persistency (`channel_exists` is a manager-table lookup), so + ## the pages released by the other context's thread get recycled before + ## the stale singleton is dereferenced. tests/persistency/test_thread_affinity + ## shows this is what turns a dormant stale ref into an observable one. + for i in 0 ..< rounds: + armSlot(s) + let id = "churn-" & $i & repeat("x", 64) + discard api.channelExists(ctx, onDone, s, id.cstring) + discard awaitSlot(s, 5_000) + +proc caseRoot(name: string): string = + getTempDir() / ("ffi_persistency_repro_" & name) + +# ── cases ─────────────────────────────────────────────────────────────── + +proc runStopSteals(api: Api, s: ptr Slot) = + ## ctx1 and ctx2 share one storage path. Stopping ctx1 runs + ## Persistency.reset(), which closes ctx2's jobs and nils the global. + let root = caseRoot("shared") + let ctx1 = createCtx(api, s, "ctx1", root, 60010, 60011) + let ctx2 = createCtx(api, s, "ctx2", root, 60020, 60021) + if failed: + return + + api.call(s, "ctx1 start_node", api.startNode, ctx1) + api.call(s, "ctx2 start_node", api.startNode, ctx2) + api.createChannel(s, "ctx2 channel_create (before ctx1 stop)", ctx2, "before") + + api.call(s, "ctx1 stop_node", api.stopNode, ctx1) + api.call(s, "ctx1 destroy", api.destroy, ctx1) + + api.createChannel(s, "ctx2 channel_create (after ctx1 stop)", ctx2, "after") + + api.call(s, "ctx2 stop_node", api.stopNode, ctx2) + api.call(s, "ctx2 destroy", api.destroy, ctx2) + +proc runDestroyOnly(api: Api, s: ptr Slot) = + ## Same, but ctx1 is destroyed without stop_node -- reset() never runs, so + ## the global keeps pointing into ctx1's released FFI-thread heap. + let root = caseRoot("shared") + let ctx1 = createCtx(api, s, "ctx1", root, 60030, 60031) + let ctx2 = createCtx(api, s, "ctx2", root, 60040, 60041) + if failed: + return + + api.call(s, "ctx1 start_node", api.startNode, ctx1) + api.call(s, "ctx2 start_node", api.startNode, ctx2) + + ## The dying context owns the singleton *and* the sds Job; ctx2 only + ## reaches for them afterwards, so its first persistency touch is already + ## against released memory. + api.createChannel(s, "ctx1 channel_create (opens the sds job)", ctx1, "owned-by-ctx1") + + api.call(s, "ctx1 destroy (no stop_node)", api.destroy, ctx1) + + api.churn(s, ctx2) + api.createChannel(s, "ctx2 channel_create (after ctx1 destroy)", ctx2, "after") + + api.call(s, "ctx2 stop_node", api.stopNode, ctx2) + api.call(s, "ctx2 destroy", api.destroy, ctx2) + +proc runTwoPaths(api: Api, s: ptr Slot) = + ## Two contexts, two storage paths. The singleton refuses to be re-targeted, + ## so the second node cannot start at all. + let rootA = caseRoot("path_a") + let rootB = caseRoot("path_b") + let ctx1 = createCtx(api, s, "ctx1", rootA, 60050, 60051) + let ctx2 = createCtx(api, s, "ctx2", rootB, 60060, 60061) + if failed: + return + + api.call(s, "ctx1 start_node", api.startNode, ctx1) + api.call(s, "ctx2 start_node (different local-storage-path)", api.startNode, ctx2) + + api.call(s, "ctx1 stop_node", api.stopNode, ctx1) + api.call(s, "ctx1 destroy", api.destroy, ctx1) + api.call(s, "ctx2 destroy", api.destroy, ctx2) + +proc runChild(which: string) = + let api = loadApi() + let s = createShared(Slot) + + case which + of CaseStopSteals: + runStopSteals(api, s) + of CaseDestroyOnly: + runDestroyOnly(api, s) + of CaseTwoPaths: + runTwoPaths(api, s) + else: + quit("unknown case " & which, 2) + + quit(if failed: 1 else: 0) + +if paramCount() >= 1 and paramStr(1).startsWith("--case-"): + runChild(paramStr(1)) + +# ── parent ────────────────────────────────────────────────────────────── + +proc runCase(flag: string): tuple[code: int, output: string] = + let logFile = getTempDir() / ("ffi_persistency_repro" & flag & ".log") + discard tryRemoveFile(logFile) + let cmd = + quoteShell(getAppFilename()) & " " & flag & " > " & quoteShell(logFile) & " 2>&1" + + let child = startProcess("/bin/sh", args = @["-c", cmd], options = {}) + let code = child.waitForExit(timeout = 300_000) + child.close() + + let output = + try: + readFile(logFile) + except IOError: + "" + discard tryRemoveFile(logFile) + return (code, output) + +proc report(flag: string, r: tuple[code: int, output: string]) = + echo "--- ", flag, " (exit ", r.code, ") ---" + for line in r.output.splitLines(): + if line.contains("[ctx") or line.contains("FAIL:") or line.contains(DisabledMarker): + echo line + +suite "FFI - persistency lifecycle across library contexts": + test "stopping one context must not disable persistence in the other": + if not fileExists(libPath()): + echo "skipped: no ", libPath() + skip() + else: + removeDir(caseRoot("shared")) + let r = runCase(CaseStopSteals) + report(CaseStopSteals, r) + removeDir(caseRoot("shared")) + + check r.code == 0 + check not r.output.contains(DisabledMarker) + + test "destroying one context must not corrupt the other's persistency": + if not fileExists(libPath()): + echo "skipped: no ", libPath() + skip() + else: + removeDir(caseRoot("shared")) + let r = runCase(CaseDestroyOnly) + report(CaseDestroyOnly, r) + removeDir(caseRoot("shared")) + + check r.code == 0 + check not r.output.contains(DisabledMarker) + + test "two contexts with different storage paths must both start": + if not fileExists(libPath()): + echo "skipped: no ", libPath() + skip() + else: + removeDir(caseRoot("path_a")) + removeDir(caseRoot("path_b")) + let r = runCase(CaseTwoPaths) + report(CaseTwoPaths, r) + removeDir(caseRoot("path_a")) + removeDir(caseRoot("path_b")) + + check r.code == 0 diff --git a/tests/persistency/test_all.nim b/tests/persistency/test_all.nim index 5b0cfdbb5..b94533aa3 100644 --- a/tests/persistency/test_all.nim +++ b/tests/persistency/test_all.nim @@ -8,3 +8,4 @@ import ./test_encoding import ./test_sds_persistency import ./test_string_lookup import ./test_singleton +import ./test_thread_affinity diff --git a/tests/persistency/test_encoding.nim b/tests/persistency/test_encoding.nim index 291616d31..543800f78 100644 --- a/tests/persistency/test_encoding.nim +++ b/tests/persistency/test_encoding.nim @@ -127,9 +127,9 @@ suite "Persistency generic encoding": removeDir(root) defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let job = p.openJob("t").get() let m = Msg( diff --git a/tests/persistency/test_facade.nim b/tests/persistency/test_facade.nim index bf22a96ba..c1253c9af 100644 --- a/tests/persistency/test_facade.nim +++ b/tests/persistency/test_facade.nim @@ -38,9 +38,9 @@ suite "Persistency facade": let root = tmpRoot("put_get") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() let k = key("c", 1'i64) @@ -57,9 +57,9 @@ suite "Persistency facade": let root = tmpRoot("batch") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() var ops: seq[TxOp] @@ -79,9 +79,9 @@ suite "Persistency facade": let root = tmpRoot("scan") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() for i in [3'i64, 1, 4, 1, 5, 9, 2]: @@ -103,9 +103,9 @@ suite "Persistency facade": let root = tmpRoot("scan_rev") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() for i in 1'i64 .. 3: @@ -123,9 +123,9 @@ suite "Persistency facade": let root = tmpRoot("delete") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() let k = key("c", 1'i64) @@ -147,9 +147,9 @@ suite "Persistency facade": let root = tmpRoot("fadel") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() let k = key("c", 1'i64) @@ -172,9 +172,9 @@ suite "Persistency facade": let root = tmpRoot("iso") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let a = p.openJob("a").get() let b = p.openJob("b").get() diff --git a/tests/persistency/test_lifecycle.nim b/tests/persistency/test_lifecycle.nim index 1abcdaccb..b2dc3a5ae 100644 --- a/tests/persistency/test_lifecycle.nim +++ b/tests/persistency/test_lifecycle.nim @@ -48,9 +48,9 @@ suite "Persistency lifecycle": defer: removeFile(marker) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() # The pre-existing file is untouched. check fileExists(marker) check readFile(marker) == "hi" @@ -60,7 +60,7 @@ suite "Persistency lifecycle": defer: removeFile(root) writeFile(root, "im a file not a dir") # collide with rootDir name - let r = Persistency.instance(root) + let r = Persistency.new(root) check r.isErr check r.error.kind == peInvalidArgument @@ -70,9 +70,9 @@ suite "Persistency lifecycle": removeDir(root) check not dirExists(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() # instance() must not have touched the filesystem check not dirExists(root) @@ -86,7 +86,7 @@ suite "Persistency lifecycle": removeFile(parent) writeFile(parent, "not a directory") let root = parent / "child" - let r = Persistency.instance(root) + let r = Persistency.new(root) check r.isErr check r.error.kind == peInvalidArgument @@ -97,20 +97,20 @@ suite "Persistency lifecycle": # First "session": write something then close. block firstSession: - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() let j = p.openJob("persist").get() await j.persistPut("msg", key("c", 1'i64), payloadBytes("v1")) let ckOk1 = await j.pollExists("msg", key("c", 1'i64)) check ckOk1 - Persistency.reset() + p.close() check fileExists(root / "persist.db") # Second "session": reopen and read the data back. block secondSession: - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let j = p.openJob("persist").get() let aw1 = await KvGet.request(j.context, "msg", key("c", 1'i64)) let got = aw1.get() @@ -121,9 +121,9 @@ suite "Persistency lifecycle": let root = tmpRoot("idem") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let a = p.openJob("same").get() let b = p.openJob("same").get() check a.id == b.id @@ -134,9 +134,9 @@ suite "Persistency lifecycle": defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("alpha").get() check t.id == "alpha" @@ -147,9 +147,9 @@ suite "Persistency lifecycle": let root = tmpRoot("rw") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t1").get() let k = key("c", 1'i64) @@ -169,9 +169,9 @@ suite "Persistency lifecycle": let root = tmpRoot("isolation") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let a = p.openJob("alpha").get() let b = p.openJob("beta").get() @@ -216,9 +216,9 @@ suite "Persistency lifecycle": let root = tmpRoot("close") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("x").get() let ctx = t.context @@ -233,9 +233,9 @@ suite "Persistency lifecycle": let root = tmpRoot("drop") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() discard p.openJob("ephemeral").get() check fileExists(root / "ephemeral.db") p.dropJob("ephemeral") @@ -245,9 +245,9 @@ suite "Persistency lifecycle": let root = tmpRoot("scan") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() var ops: seq[TxOp] @@ -275,9 +275,9 @@ suite "Persistency lifecycle": let root = tmpRoot("delete") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let t = p.openJob("t").get() let k = key("d", 1'i64) diff --git a/tests/persistency/test_sds_persistency.nim b/tests/persistency/test_sds_persistency.nim index 4a955310d..a922f6e06 100644 --- a/tests/persistency/test_sds_persistency.nim +++ b/tests/persistency/test_sds_persistency.nim @@ -56,9 +56,9 @@ suite "SDS persistency adapter (0.3.0 snapshot model)": let root = tmpRoot("roundtrip") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let job = p.openJob("sds").get() let persistence = newSdsPersistence(job) let channelId = "chan-1".SdsChannelID @@ -86,9 +86,9 @@ suite "SDS persistency adapter (0.3.0 snapshot model)": let root = tmpRoot("empty") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let job = p.openJob("sds").get() let persistence = newSdsPersistence(job) @@ -102,9 +102,9 @@ suite "SDS persistency adapter (0.3.0 snapshot model)": let root = tmpRoot("evict") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let job = p.openJob("sds").get() let persistence = newSdsPersistence(job) let channelId = "c".SdsChannelID @@ -129,9 +129,9 @@ suite "SDS persistency adapter (0.3.0 snapshot model)": let root = tmpRoot("drop") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let job = p.openJob("sds").get() let persistence = newSdsPersistence(job) let channelId = "d".SdsChannelID diff --git a/tests/persistency/test_singleton.nim b/tests/persistency/test_singleton.nim index 01542f409..9b7f0437e 100644 --- a/tests/persistency/test_singleton.nim +++ b/tests/persistency/test_singleton.nim @@ -3,77 +3,109 @@ import std/[os, strutils, times] import chronos, results import testutils/unittests -import brokers/multi_request_broker +import brokers/[request_broker, broker_context] import logos_delivery/waku/persistency/persistency proc tmpRoot(label: string): string = - let p = getTempDir() / ("persistency_singleton_" & label & "_" & $epochTime().int) + let p = getTempDir() / ("persistency_instance_" & label & "_" & $epochTime().int) removeDir(p) p -suite "Persistency singleton": - test "instance(rootDir) is idempotent with the same rootDir": - let root = tmpRoot("idem") - defer: - removeDir(root) - defer: - Persistency.reset() - - let p1 = Persistency.instance(root).get() - let p2 = Persistency.instance(root).get() - check p1 == p2 - - test "instance(rootDir) refuses re-init with a different rootDir": +suite "Persistency instances": + test "new(rootDir) builds independent instances": let rootA = tmpRoot("a") let rootB = tmpRoot("b") defer: removeDir(rootA) defer: removeDir(rootB) + + let pA = Persistency.new(rootA).get() + let pB = Persistency.new(rootB).get() defer: - Persistency.reset() + pA.close() + defer: + pB.close() - discard Persistency.instance(rootA).get() - let r = Persistency.instance(rootB) - check r.isErr - check r.error.kind == peInvalidArgument + check pA.rootDir == rootA + check pB.rootDir == rootB + check pA != pB - test "no-arg instance() fails before init, succeeds after": - let root = tmpRoot("noarg") + test "new(rootDir) with the same rootDir yields distinct instances": + let root = tmpRoot("same") defer: removeDir(root) + + let p1 = Persistency.new(root).get() + let p2 = Persistency.new(root).get() defer: - Persistency.reset() + p1.close() + defer: + p2.close() - let before = Persistency.instance() - check before.isErr - check before.error.kind == peClosed + check p1 != p2 - discard Persistency.instance(root).get() - let after = Persistency.instance() - check after.isOk + test "close() is idempotent": + let root = tmpRoot("close") + defer: + removeDir(root) - test "reset() makes the next instance() target a different rootDir": - let rootA = tmpRoot("rs-a") - let rootB = tmpRoot("rs-b") + let p = Persistency.new(root).get() + discard p.openJob("j").get() + p.close() + p.close() + check not p.hasJob("j") + +suite "GetPersistency broker": + test "request fails when no provider is installed": + let ctx = NewBrokerContext() + check GetPersistency.request(ctx).isErr + + test "request returns the provided instance, clearProvider removes it": + let root = tmpRoot("broker") + defer: + removeDir(root) + + let ctx = NewBrokerContext() + let p = Persistency.new(root).get() + defer: + p.close() + + discard GetPersistency.reprovideIt(ctx): + ok(p) + + let got = GetPersistency.request(ctx) + check got.isOk + check got.get() == p + + GetPersistency.clearProvider(ctx) + check GetPersistency.request(ctx).isErr + + test "providers on different contexts resolve different instances": + let rootA = tmpRoot("ctx-a") + let rootB = tmpRoot("ctx-b") defer: removeDir(rootA) defer: removeDir(rootB) + + let ctxA = NewBrokerContext() + let ctxB = NewBrokerContext() + let pA = Persistency.new(rootA).get() + let pB = Persistency.new(rootB).get() defer: - Persistency.reset() - - let pA = Persistency.instance(rootA).get() - check pA.rootDir == rootA - Persistency.reset() - - let pB = Persistency.instance(rootB).get() - check pB.rootDir == rootB - check pA != pB - - test "reset() is idempotent": + pA.close() defer: - Persistency.reset() - Persistency.reset() - Persistency.reset() - check Persistency.instance().isErr + pB.close() + + discard GetPersistency.reprovideIt(ctxA): + ok(pA) + discard GetPersistency.reprovideIt(ctxB): + ok(pB) + defer: + GetPersistency.clearProvider(ctxA) + defer: + GetPersistency.clearProvider(ctxB) + + check GetPersistency.request(ctxA).get() == pA + check GetPersistency.request(ctxB).get() == pB diff --git a/tests/persistency/test_string_lookup.nim b/tests/persistency/test_string_lookup.nim index b8d1ff7b0..53a112230 100644 --- a/tests/persistency/test_string_lookup.nim +++ b/tests/persistency/test_string_lookup.nim @@ -38,9 +38,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("notfound") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let r = p.job("nope") check r.isErr @@ -50,9 +50,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("found") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let opened = p.openJob("alpha").get() let looked = p.job("alpha").get() @@ -63,9 +63,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("has") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() check not p.hasJob("x") discard p.openJob("x") @@ -77,9 +77,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("subscript") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() discard p.openJob("a").get() let j = p["a"] check j.id == "a" @@ -88,9 +88,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("rw") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() discard p.openJob("svc").get() let k = key("c", 1'i64) @@ -107,9 +107,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("missingread") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let g = await p.get("nope", "msg", key("k")) check g.isErr @@ -127,9 +127,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("missingwrite") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() # Should not raise and should not leak any state. await p.persistPut("ghost", "msg", key("k"), payloadBytes("v")) @@ -145,9 +145,9 @@ suite "Persistency string-id lookup": tag: string n: int64 - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() discard p.openJob("e").get() let k = key("items", 1'i64) @@ -164,9 +164,9 @@ suite "Persistency string-id lookup": let root = tmpRoot("scan") defer: removeDir(root) - let p = Persistency.instance(root).get() + let p = Persistency.new(root).get() defer: - Persistency.reset() + p.close() let j = p.openJob("s").get() for i in 1'i64 .. 3: diff --git a/tests/persistency/test_thread_affinity.nim b/tests/persistency/test_thread_affinity.nim new file mode 100644 index 000000000..2840e6842 --- /dev/null +++ b/tests/persistency/test_thread_affinity.nim @@ -0,0 +1,144 @@ +{.used.} + +## Regression guard for the former singleton thread-affinity defect +## (channel_lifecycle.sdsPersistence -> openJob -> tables.rawGet SIGSEGV): +## a process-global `gPersistency` was allocated on whichever FFI thread +## first touched it, and under `--mm:refc` outlived that thread's heap. +## +## Post-refactor semantics under test: +## * a `Persistency` instance is created, used and closed entirely on +## its owning thread -- nothing is process-global anymore. The owner +## drives two in-memory jobs end to end: two storage workers are +## spun up, written to and read back through their broker contexts, +## then torn down (one via `closeJob`, the rest via `close`) before +## the owning thread exits. +## * the `GetPersistency` broker is context- AND thread-scoped +## (single-thread RequestBroker registries are threadvars), so a +## second thread cannot reach the owner's instance -- not even when +## it knows the owner's `BrokerContext` id. + +import std/times +import chronos, results +import testutils/unittests +import brokers/[request_broker, broker_context] +import logos_delivery/waku/persistency/persistency + +var + ownerCtx: BrokerContext + ## distinct uint32 (POD); safe to hand across threads -- reading it + ## from the second thread is exactly the escape hatch under test. + ownerFailed: bool + userReachedInstance: bool + ## Plain bools (no GC'd payload) so the worker threads can set them; + ## joinThread orders the writes before the main-thread checks. + +proc payload(s: string): seq[byte] = + result = newSeq[byte](s.len) + for i, c in s: + result[i] = byte(c) + +# Bounded poll on exists() to bridge the documented persist->read race. +proc waitUntilExists( + t: Job, category: string, k: Key, timeoutMs = 1000 +): Future[bool] {.async.} = + let deadline = epochTime() + (timeoutMs.float / 1000.0) + while epochTime() < deadline: + let r = await t.exists(category, k) + if r.isOk and r.get(): + return true + await sleepAsync(chronos.milliseconds(2)) + return false + +proc fail(msg: string) = + echo " FAIL: ", msg + ownerFailed = true + +proc exerciseJobs(p: Persistency) {.async.} = + ## Two jobs => two storage worker threads, each with a private + ## in-memory database. Round-trip a write through each, prove they are + ## independent, then tear one down explicitly. + let a = p.openJob("a").expect("openJob a") + let b = p.openJob("b").expect("openJob b") + if a.context == b.context: + fail("jobs must run under distinct broker contexts") + + let k = key("aff", 1'i64) + await a.persistPut("msg", k, payload("via-a")) + await b.persistPut("msg", k, payload("via-b")) + + if not await a.waitUntilExists("msg", k): + fail("job a never saw its own write") + if not await b.waitUntilExists("msg", k): + fail("job b never saw its own write") + + ## Private per-worker :memory: databases: same key, different payloads. + let fromA = (await a.get("msg", k)).expect("get via a") + let fromB = (await b.get("msg", k)).expect("get via b") + if fromA.isNone or fromA.get() != payload("via-a"): + fail("job a returned the wrong payload") + if fromB.isNone or fromB.get() != payload("via-b"): + fail("job b returned the wrong payload") + + ## Explicit single-job teardown joins its worker thread. + p.closeJob("a") + if p.hasJob("a"): + fail("job a still open after closeJob") + if not p.hasJob("b"): + fail("job b must survive closing job a") + + ## The surviving worker still answers after a sibling teardown. + if not await b.waitUntilExists("msg", k): + fail("job b stopped answering after job a closed") + +proc ownerThread(unused: int) {.thread.} = + ## Stands in for the FFI thread owning a node: create the instance, + ## exercise its jobs, provide it under this thread's context, resolve + ## it via the broker, then tear everything down before the thread exits. + let ctx = NewBrokerContext() + ownerCtx = ctx + + let p = Persistency.new(InMemoryStoragePath).expect("Persistency.new on owner thread") + + try: + waitFor exerciseJobs(p) + except CatchableError as e: + fail("exerciseJobs raised: " & e.msg) + + discard GetPersistency.reprovideIt(ctx): + ok(p) + + let r = GetPersistency.request(ctx) + if r.isErr() or r.get() != p or not r.get().hasJob("b"): + fail("same-thread broker resolution broken") + + GetPersistency.clearProvider(ctx) + ## Joins job b's worker thread; owner heap objects die with this thread. + p.close() + if p.hasJob("b"): + fail("job b still open after close") + +proc userThread(unused: int) {.thread.} = + ## Stands in for a second FFI thread with a fresh heap. There must be + ## no path to the first thread's instance: neither via its context id + ## nor via any fresh context. + let viaOwnerCtx = GetPersistency.request(ownerCtx) + if viaOwnerCtx.isOk(): + echo " FAIL: owner's instance reachable from another thread via its ctx" + userReachedInstance = true + + let viaFreshCtx = GetPersistency.request(NewBrokerContext()) + if viaFreshCtx.isOk(): + echo " FAIL: an instance is reachable through a fresh context" + userReachedInstance = true + +suite "Persistency - thread affinity": + test "instance, jobs and broker provider are confined to the owning thread": + var owner: Thread[int] + createThread(owner, ownerThread, 0) + joinThread(owner) + check not ownerFailed + + var user: Thread[int] + createThread(user, userThread, 0) + joinThread(user) + check not userReachedInstance