Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -115,45 +115,45 @@ build-libsds-for-android-arch: NIM_PARAMS := $(NIM_PARAMS) --passC="-I$(ANDROID_
build-libsds-for-android-arch: NIM_PARAMS := $(NIM_PARAMS) --passL="-L$(ANDROID_TOOLCHAIN_DIR)/sysroot/usr/lib/$(ARCH_DIRNAME)/$(ANDROID_TARGET)"
build-libsds-for-android-arch:
CC=$(ANDROID_TOOLCHAIN_DIR)/bin/$(ANDROID_ARCH)$(ANDROID_TARGET)-clang \
CPU=$(CPU) ABIDIR=$(ABIDIR) \
ARCH=$(ARCH) ABIDIR=$(ABIDIR) \
ARCH_DIRNAME=$(ARCH_DIRNAME) \
ANDROID_ARCH=$(ANDROID_ARCH) \
ANDROID_TOOLCHAIN_DIR=$(ANDROID_TOOLCHAIN_DIR) \
$(ENV_SCRIPT) \
nim libsdsAndroid $(NIM_PARAMS) sds.nims

libsds-android-arm64: ANDROID_ARCH=aarch64-linux-android
libsds-android-arm64: CPU=arm64
libsds-android-arm64: ARCH=arm64
libsds-android-arm64: ABIDIR=arm64-v8a
libsds-android-arm64: ARCH_DIRNAME=aarch64-linux-android
libsds-android-arm64: | libsds-android-precheck build deps
$(MAKE) build-libsds-for-android-arch ANDROID_ARCH=$(ANDROID_ARCH) \
CPU=$(CPU) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME)
ARCH=$(ARCH) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME)

libsds-android-amd64: ANDROID_ARCH=x86_64-linux-android
libsds-android-amd64: CPU=amd64
libsds-android-amd64: ARCH=amd64
libsds-android-amd64: ABIDIR=x86_64
libsds-android-amd64: ARCH_DIRNAME=x86_64-linux-android
libsds-android-amd64: | libsds-android-precheck build deps
$(MAKE) build-libsds-for-android-arch ANDROID_ARCH=$(ANDROID_ARCH) \
CPU=$(CPU) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME)
ARCH=$(ARCH) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME)

libsds-android-x86: ANDROID_ARCH=i686-linux-android
libsds-android-x86: CPU=i386
libsds-android-x86: ARCH=i386
libsds-android-x86: ABIDIR=x86
libsds-android-x86: ARCH_DIRNAME=i686-linux-android
libsds-android-x86: | libsds-android-precheck build deps
$(MAKE) build-libsds-for-android-arch ANDROID_ARCH=$(ANDROID_ARCH) \
CPU=$(CPU) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME)
ARCH=$(ARCH) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME)

libsds-android-arm: ANDROID_ARCH=armv7a-linux-androideabi
libsds-android-arm: CPU=arm
libsds-android-arm: ARCH=arm
libsds-android-arm: ABIDIR=armeabi-v7a
libsds-android-arm: ARCH_DIRNAME=arm-linux-androideabi
libsds-android-arm: | libsds-android-precheck build deps
# cross-rs target architecture name does not match the one used in android
$(MAKE) build-libsds-for-android-arch ANDROID_ARCH=$(ANDROID_ARCH) \
CPU=$(CPU) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME) \
ARCH=$(ARCH) ABIDIR=$(ABIDIR) ARCH_DIRNAME=$(ARCH_DIRNAME) \

libsds-android:
ifeq ($(ARCH),arm64)
Expand Down
48 changes: 38 additions & 10 deletions library/libsds.nim
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
when defined(linux):
{.passl: "-Wl,-soname,libsds.so".}

import std/[typetraits, tables, atomics], chronos, chronicles
import std/[typetraits, tables, atomics, locks], chronos, chronicles
import
./sds_thread/sds_thread,
./alloc,
Expand Down Expand Up @@ -57,6 +57,29 @@ template callEventCallback(ctx: ptr SdsContext, eventName: string, body: untyped
RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), ctx[].eventUserData
)

var
ctxPool: seq[ptr SdsContext]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seq cannot be used cross-thread until we migrate to the orc memory manager meaning that this list needs to be managed manually - the absolutely easiest way to do that is to use an array + counter and limit the number of sds contexts that can be created.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ctxPool: seq[ptr SdsContext]
ctxPool: array[32, ptr SdsContext]
ctxPos: int

something like this - assuming you don't want more than 32 concurrent contexts

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay for now to allow as much as needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how many is needed. And eventually we'll need a proper fix anyway

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I mean using a seq here is undefined behavior - if it grows more than once, it'll crash - seqs are tied to a particular thread, arrays are not - for a cross-thread seq, one needs to use createShared and a bit of extra work (similar to sharedseq in waku / ffi)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tested this to work for now in Go tests and Status App.
We agreed with @Ivansete-status to sort this out in next releases.

ctxPoolLock: Lock

proc acquireCtx(callback: SdsCallBack, userData: pointer): ptr SdsContext =
ctxPoolLock.acquire()
defer: ctxPoolLock.release()
if ctxPool.len > 0:
result = ctxPool.pop()
else:
result = sds_thread.createSdsThread().valueOr:
let msg = "Error in createSdsThread: " & $error
callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData)
return nil

proc releaseCtx(ctx: ptr SdsContext) =
ctxPoolLock.acquire()
defer: ctxPoolLock.release()
ctx.userData = nil
ctx.eventCallback = nil
ctx.eventUserData = nil
ctxPool.add(ctx)

proc handleRequest(
ctx: ptr SdsContext,
requestType: RequestType,
Expand Down Expand Up @@ -140,10 +163,9 @@ proc SdsNewReliabilityManager(
echo "error: missing callback in NewReliabilityManager"
return nil

## Create the SDS thread that will keep waiting for req from the main thread.
var ctx = sds_thread.createSdsThread().valueOr:
let msg = "Error in createSdsThread: " & $error
callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData)
## Create or reuse the SDS thread that will keep waiting for req from the main thread.
var ctx = acquireCtx(callback, userData)
if ctx.isNil():
return nil

ctx.userData = userData
Expand Down Expand Up @@ -183,14 +205,20 @@ proc SdsCleanupReliabilityManager(
initializeLibrary()
checkLibsdsParams(ctx, callback, userData)

sds_thread.destroySdsThread(ctx).isOkOr:
let msg = "libsds error: " & $error
callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData)
let resetRes = handleRequest(
ctx,
RequestType.LIFECYCLE,
SdsLifecycleRequest.createShared(SdsLifecycleMsgType.RESET_RELIABILITY_MANAGER),
callback,
userData,
)

if resetRes == RET_ERR:
return RET_ERR

## always need to invoke the callback although we don't retrieve value to the caller
callback(RET_OK, nil, 0, userData)
releaseCtx(ctx)

# handleRequest already invoked the callback; nothing else to signal here.
return RET_OK

proc SdsResetReliabilityManager(
Expand Down