From 0b8b037adff17cd8c293bae1c72509f49d09f349 Mon Sep 17 00:00:00 2001 From: Ivan FB Date: Fri, 12 Jun 2026 10:05:18 +0200 Subject: [PATCH] fix: async Cleanup for recycle ABI + guard rmRegistry sds_destroy is now non-blocking and recycles the context, delivering the drain outcome via the callback. Cleanup blocks on that callback through sdsCall, which also backpressures NewReliabilityManager against recycle so the fixed context pool cannot be exhausted under create/destroy churn. Guard rmRegistry with a RWMutex: it is written by register/unregister (Create/Cleanup) and read by sdsGlobalEventCallback on the nim-ffi event thread, which otherwise triggered a fatal "concurrent map read and map write" once multiple managers ran concurrently. Co-Authored-By: Claude Opus 4.8 --- sds/sds.go | 17 ++++++++++++----- sds/sds_common.go | 27 +++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/sds/sds.go b/sds/sds.go index d3ec50d..05a8374 100644 --- a/sds/sds.go +++ b/sds/sds.go @@ -81,8 +81,8 @@ package sds return sds_create((const uint8_t*) reqCbor, reqCborLen, (SdsCallBack) SdsGoCallback, resp); } - static int cGoSdsDestroy(void* ctx) { - return sds_destroy(ctx); + static int cGoSdsDestroy(void* ctx, void* resp) { + return sds_destroy(ctx, (SdsCallBack) SdsGoCallback, resp); } static unsigned long long cGoSdsAddEventListener(void* ctx, const char* eventName) { @@ -269,7 +269,7 @@ func sdsGlobalEventCallback(callerRet C.int, msg *C.char, len C.size_t, userData eventCbor = C.GoBytes(unsafe.Pointer(msg), C.int(len)) } - rm, ok := rmRegistry[userData] // userData carries the rm's ctx handle + rm, ok := lookupReliabilityManager(userData) // userData carries the rm's ctx handle if !ok { return } @@ -288,9 +288,16 @@ func (rm *ReliabilityManager) Cleanup() error { rm.logger.Debug("cleaning up reliability manager") - ret := int(C.cGoSdsDestroy(rm.rmCtx)) + // sds_destroy recycles the context (it does not stop the worker threads). + // It returns immediately; the real outcome is delivered through the callback + // once the FFI thread has drained and parked the context, so we block on it + // via sdsCall. Blocking here also backpressures Create against recycle so the + // fixed context pool can't be exhausted under churn. + ret, data := sdsCall(func(resp unsafe.Pointer) { + C.cGoSdsDestroy(rm.rmCtx, resp) + }) if ret != C.RET_OK { - return errorspkg.Errorf("error CleanupReliabilityManager: ret code %d", ret) + return respError("error CleanupReliabilityManager", ret, data) } unregisterReliabilityManager(rm) diff --git a/sds/sds_common.go b/sds/sds_common.go index adf2e88..92c642c 100644 --- a/sds/sds_common.go +++ b/sds/sds_common.go @@ -1,6 +1,7 @@ package sds import ( + "sync" "time" "unsafe" @@ -29,24 +30,42 @@ type ReliabilityManager struct { // rm is the event being emited for. Since we only have a global // callback in the go side, We register all the rm's that we create // so we can later obtain which instance of `ReliabilityManager` it should -// be invoked depending on the ctx received -var rmRegistry map[unsafe.Pointer]*ReliabilityManager +// be invoked depending on the ctx received. +// +// rmRegistryMu guards the map: it is written by register/unregister (on +// Create/Cleanup) and read by sdsGlobalEventCallback on the nim-ffi event +// thread, so concurrent managers would otherwise trigger a fatal +// "concurrent map read and map write". +var ( + rmRegistryMu sync.RWMutex + rmRegistry map[unsafe.Pointer]*ReliabilityManager +) func init() { rmRegistry = make(map[unsafe.Pointer]*ReliabilityManager) } func registerReliabilityManager(rm *ReliabilityManager) { - _, ok := rmRegistry[rm.rmCtx] - if !ok { + rmRegistryMu.Lock() + defer rmRegistryMu.Unlock() + if _, ok := rmRegistry[rm.rmCtx]; !ok { rmRegistry[rm.rmCtx] = rm } } func unregisterReliabilityManager(rm *ReliabilityManager) { + rmRegistryMu.Lock() + defer rmRegistryMu.Unlock() delete(rmRegistry, rm.rmCtx) } +func lookupReliabilityManager(ctx unsafe.Pointer) (*ReliabilityManager, bool) { + rmRegistryMu.RLock() + defer rmRegistryMu.RUnlock() + rm, ok := rmRegistry[ctx] + return rm, ok +} + // sdsEventEnvelope is the CBOR wrapper libsds emits for every event: // { eventType: , payload: }. type sdsEventEnvelope struct {