From 3b1c2f8fc4f3d45d8147962f6142ad99f9d02a21 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 11:23:12 +0400 Subject: [PATCH 1/9] Fix usage of req after free --- library/storage_context.nim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/storage_context.nim b/library/storage_context.nim index c5eddd2b..f2074957 100644 --- a/library/storage_context.nim +++ b/library/storage_context.nim @@ -94,8 +94,9 @@ proc sendRequestToStorageThread*( # Send the request to the Logos Storage thread let sentOk = ctx.reqChannel.trySend(req) if not sentOk: + let reqDesc = $req[] deallocShared(req) - return err("Failed to send request to the Logos Storage thread: " & $req[]) + return err("Failed to send request to the Logos Storage thread: " & reqDesc) # Notify the Logos Storage thread that a request is available let fireSyncRes = ctx.reqSignal.fireSync() From c630224a59824b421e97e0ceda7311c4bccf6923 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 11:39:32 +0400 Subject: [PATCH 2/9] Do not deallocShared after successfull trySend --- library/storage_context.nim | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/storage_context.nim b/library/storage_context.nim index f2074957..df352bdf 100644 --- a/library/storage_context.nim +++ b/library/storage_context.nim @@ -37,7 +37,7 @@ type StorageContext* = object # To notify the Logos Storage thread that a request is ready reqSignal: ThreadSignalPtr - # To notify the client thread that the request was received. + # To notify the client thread that the request was received. # It is acknowledgment signal (handshake). reqReceivedSignal: ThreadSignalPtr @@ -48,7 +48,7 @@ type StorageContext* = object # Function called by the library to notify the client of global events eventCallback*: pointer - # Custom state attached by the client to the context, + # Custom state attached by the client to the context, # returned with every event callback eventUserData*: pointer @@ -56,8 +56,8 @@ type StorageContext* = object running: Atomic[bool] template callEventCallback(ctx: ptr StorageContext, eventName: string, body: untyped) = - ## Template used to notify the client of global events - ## Example: onConnectionChanged, onProofMissing, etc. + ## Template used to notify the client of global events + ## Example: onConnectionChanged, onProofMissing, etc. if isNil(ctx[].eventCallback): error eventName & " - eventCallback is nil" return @@ -98,24 +98,24 @@ proc sendRequestToStorageThread*( deallocShared(req) return err("Failed to send request to the Logos Storage thread: " & reqDesc) + # trySend has succeeded: req is published in the channel and + # owned by the Logos Storage thread, which frees it in handleRes. + # Notify the Logos Storage thread that a request is available let fireSyncRes = ctx.reqSignal.fireSync() if fireSyncRes.isErr(): - deallocShared(req) return err( "Failed to send request to the Logos Storage thread: unable to fireSync: " & $fireSyncRes.error ) if fireSyncRes.get() == false: - deallocShared(req) return err("Failed to send request to the Logos Storage thread: fireSync timed out.") # Wait until the Logos Storage thread properly received the request let res = ctx.reqReceivedSignal.waitSync(timeout) if res.isErr(): - deallocShared(req) return err( "Failed to send request to the Logos Storage thread: unable to receive reqReceivedSignal signal." ) @@ -173,13 +173,13 @@ proc createStorageContext*(): Result[ptr StorageContext, string] = # Allocates a StorageContext in shared memory (for the main thread) var ctx = createShared(StorageContext, 1) - # This signal is used by the main side to wake the Logos Storage thread + # This signal is used by the main side to wake the Logos Storage thread # when a new request is enqueued. ctx.reqSignal = ThreadSignalPtr.new().valueOr: return err("Failed to create a context: unable to create reqSignal ThreadSignalPtr.") - # Used to let the caller know that the Logos Storage thread has + # Used to let the caller know that the Logos Storage thread has # acknowledged / picked up a request (like a handshake). ctx.reqReceivedSignal = ThreadSignalPtr.new().valueOr: return err( From aef4fcc3d467df308379e89dd9dde9c1a23e2ce7 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 11:49:40 +0400 Subject: [PATCH 3/9] Free the reqContent when the request was not sent to the thread --- library/storage_context.nim | 2 +- .../requests/node_debug_request.nim | 2 +- .../requests/node_download_request.nim | 2 +- .../requests/node_info_request.nim | 2 +- .../requests/node_lifecycle_request.nim | 2 +- .../requests/node_p2p_request.nim | 2 +- .../requests/node_storage_request.nim | 2 +- .../requests/node_upload_request.nim | 2 +- .../storage_thread_request.nim | 21 +++++++++++++++++++ 9 files changed, 29 insertions(+), 8 deletions(-) diff --git a/library/storage_context.nim b/library/storage_context.nim index df352bdf..3f412ab3 100644 --- a/library/storage_context.nim +++ b/library/storage_context.nim @@ -95,7 +95,7 @@ proc sendRequestToStorageThread*( let sentOk = ctx.reqChannel.trySend(req) if not sentOk: let reqDesc = $req[] - deallocShared(req) + req.destroy() return err("Failed to send request to the Logos Storage thread: " & reqDesc) # trySend has succeeded: req is published in the channel and diff --git a/library/storage_thread_requests/requests/node_debug_request.nim b/library/storage_thread_requests/requests/node_debug_request.nim index 8bf3106c..6c8f95aa 100644 --- a/library/storage_thread_requests/requests/node_debug_request.nim +++ b/library/storage_thread_requests/requests/node_debug_request.nim @@ -41,7 +41,7 @@ proc createShared*( ret[].logLevel = logLevel.alloc() return ret -proc destroyShared(self: ptr NodeDebugRequest) = +proc destroyShared*(self: ptr NodeDebugRequest) = deallocShared(self[].peerId) deallocShared(self[].logLevel) deallocShared(self) diff --git a/library/storage_thread_requests/requests/node_download_request.nim b/library/storage_thread_requests/requests/node_download_request.nim index f0297bbe..3f1938f5 100644 --- a/library/storage_thread_requests/requests/node_download_request.nim +++ b/library/storage_thread_requests/requests/node_download_request.nim @@ -72,7 +72,7 @@ proc createShared*( return ret -proc destroyShared(self: ptr NodeDownloadRequest) = +proc destroyShared*(self: ptr NodeDownloadRequest) = deallocShared(self[].cid) deallocShared(self[].filepath) deallocShared(self) diff --git a/library/storage_thread_requests/requests/node_info_request.nim b/library/storage_thread_requests/requests/node_info_request.nim index 7e755a3a..a07ffc32 100644 --- a/library/storage_thread_requests/requests/node_info_request.nim +++ b/library/storage_thread_requests/requests/node_info_request.nim @@ -27,7 +27,7 @@ proc createShared*(T: type NodeInfoRequest, op: NodeInfoMsgType): ptr type T = ret[].operation = op return ret -proc destroyShared(self: ptr NodeInfoRequest) = +proc destroyShared*(self: ptr NodeInfoRequest) = deallocShared(self) proc getRepo( diff --git a/library/storage_thread_requests/requests/node_lifecycle_request.nim b/library/storage_thread_requests/requests/node_lifecycle_request.nim index cccfd3fb..0f0125ac 100644 --- a/library/storage_thread_requests/requests/node_lifecycle_request.nim +++ b/library/storage_thread_requests/requests/node_lifecycle_request.nim @@ -90,7 +90,7 @@ proc createShared*( ret[].configJson = configJson.alloc() return ret -proc destroyShared(self: ptr NodeLifecycleRequest) = +proc destroyShared*(self: ptr NodeLifecycleRequest) = deallocShared(self[].configJson) deallocShared(self) diff --git a/library/storage_thread_requests/requests/node_p2p_request.nim b/library/storage_thread_requests/requests/node_p2p_request.nim index bfc29b88..ef9737bb 100644 --- a/library/storage_thread_requests/requests/node_p2p_request.nim +++ b/library/storage_thread_requests/requests/node_p2p_request.nim @@ -35,7 +35,7 @@ proc createShared*( ret[].peerAddresses = peerAddresses return ret -proc destroyShared(self: ptr NodeP2PRequest) = +proc destroyShared*(self: ptr NodeP2PRequest) = deallocShared(self[].peerId) deallocShared(self) diff --git a/library/storage_thread_requests/requests/node_storage_request.nim b/library/storage_thread_requests/requests/node_storage_request.nim index b2343041..8c44f172 100644 --- a/library/storage_thread_requests/requests/node_storage_request.nim +++ b/library/storage_thread_requests/requests/node_storage_request.nim @@ -50,7 +50,7 @@ proc createShared*( return ret -proc destroyShared(self: ptr NodeStorageRequest) = +proc destroyShared*(self: ptr NodeStorageRequest) = deallocShared(self[].cid) deallocShared(self) diff --git a/library/storage_thread_requests/requests/node_upload_request.nim b/library/storage_thread_requests/requests/node_upload_request.nim index 064439b2..a10ada32 100644 --- a/library/storage_thread_requests/requests/node_upload_request.nim +++ b/library/storage_thread_requests/requests/node_upload_request.nim @@ -80,7 +80,7 @@ proc createShared*( return ret -proc destroyShared(self: ptr NodeUploadRequest) = +proc destroyShared*(self: ptr NodeUploadRequest) = deallocShared(self[].filepath) deallocShared(self[].sessionId) deallocShared(self) diff --git a/library/storage_thread_requests/storage_thread_request.nim b/library/storage_thread_requests/storage_thread_request.nim index eaa0bba4..a262356e 100644 --- a/library/storage_thread_requests/storage_thread_request.nim +++ b/library/storage_thread_requests/storage_thread_request.nim @@ -52,6 +52,27 @@ proc createShared*( ret[].userData = userData return ret +proc destroy*(request: ptr StorageThreadRequest) = + ## Frees the payload (reqContent) and the wrapper on error paths, + ## when the request is not handed to the storage thread. + ## On success paths, the storage thread frees the payload and the wrapper. + case request[].reqType + of LIFECYCLE: + cast[ptr NodeLifecycleRequest](request[].reqContent).destroyShared() + of INFO: + cast[ptr NodeInfoRequest](request[].reqContent).destroyShared() + of RequestType.DEBUG: + cast[ptr NodeDebugRequest](request[].reqContent).destroyShared() + of P2P: + cast[ptr NodeP2PRequest](request[].reqContent).destroyShared() + of UPLOAD: + cast[ptr NodeUploadRequest](request[].reqContent).destroyShared() + of DOWNLOAD: + cast[ptr NodeDownloadRequest](request[].reqContent).destroyShared() + of STORAGE: + cast[ptr NodeStorageRequest](request[].reqContent).destroyShared() + deallocShared(request) + # NOTE: User callbacks are executed on the working thread. # They must be fast and non-blocking; otherwise this thread will be blocked # and no further requests can be processed. From afc51ad5c7c2b1e0e2cce6cc105221b89689ccac Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 20:03:03 +0400 Subject: [PATCH 4/9] Use a pointer to free the memory in destroyShared --- library/libstorage.nim | 5 +--- .../requests/node_upload_request.nim | 25 ++++++++++++++++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/library/libstorage.nim b/library/libstorage.nim index e41e52dd..3481e166 100644 --- a/library/libstorage.nim +++ b/library/libstorage.nim @@ -296,11 +296,8 @@ proc storage_upload_chunk( initializeLibrary() checkLibstorageParams(ctx, callback, userData) - let chunk = newSeq[byte](len) - copyMem(addr chunk[0], data, len) - let reqContent = NodeUploadRequest.createShared( - NodeUploadMsgType.CHUNK, sessionId = sessionId, chunk = chunk + NodeUploadMsgType.CHUNK, sessionId = sessionId, chunkData = data, chunkLen = len.int ) let res = storage_context.sendRequestToStorageThread( ctx, RequestType.UPLOAD, reqContent, callback, userData diff --git a/library/storage_thread_requests/requests/node_upload_request.nim b/library/storage_thread_requests/requests/node_upload_request.nim index a10ada32..b2bd94a5 100644 --- a/library/storage_thread_requests/requests/node_upload_request.nim +++ b/library/storage_thread_requests/requests/node_upload_request.nim @@ -47,7 +47,9 @@ type NodeUploadRequest* = object operation: NodeUploadMsgType sessionId: cstring filepath: cstring - chunk: seq[byte] + # Use a pointer to free the memory in destroyShared + chunk: pointer + chunkLen: int chunkSize: csize_t type @@ -68,21 +70,30 @@ proc createShared*( op: NodeUploadMsgType, sessionId: cstring = "", filepath: cstring = "", - chunk: seq[byte] = @[], + chunkData: ptr byte = nil, + chunkLen: int = 0, chunkSize: csize_t = 0, ): ptr type T = var ret = createShared(T) ret[].operation = op ret[].sessionId = sessionId.alloc() ret[].filepath = filepath.alloc() - ret[].chunk = chunk + ret[].chunkLen = chunkLen ret[].chunkSize = chunkSize + if chunkLen > 0 and chunkData != nil: + ret[].chunk = allocShared(chunkLen) + copyMem(ret[].chunk, chunkData, chunkLen) + return ret proc destroyShared*(self: ptr NodeUploadRequest) = deallocShared(self[].filepath) deallocShared(self[].sessionId) + + if self[].chunk != nil: + deallocShared(self[].chunk) + deallocShared(self) proc init( @@ -347,7 +358,13 @@ proc process*( return err($res.error) return res of NodeUploadMsgType.CHUNK: - let res = (await chunk(storage, self.sessionId, self.chunk)) + # self.chunk is a raw memory pointer, but chunk() takes a seq[byte], + # so we copy the bytes into a new seq. + var data = newSeq[byte](self.chunkLen) + if self.chunkLen > 0: + copyMem(addr data[0], self.chunk, self.chunkLen) + + let res = (await chunk(storage, self.sessionId, data)) if res.isErr: error "Failed to CHUNK.", error = res.error return err($res.error) From e0acc97effdc48d1fdc5859a0af8cc39c84c7238 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 21:03:35 +0400 Subject: [PATCH 5/9] Use a pointer to free the memory in destroyShared --- .../requests/node_p2p_request.nim | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/library/storage_thread_requests/requests/node_p2p_request.nim b/library/storage_thread_requests/requests/node_p2p_request.nim index ef9737bb..44a45e40 100644 --- a/library/storage_thread_requests/requests/node_p2p_request.nim +++ b/library/storage_thread_requests/requests/node_p2p_request.nim @@ -21,7 +21,9 @@ type NodeP2PMsgType* = enum type NodeP2PRequest* = object operation: NodeP2PMsgType peerId: cstring - peerAddresses: seq[cstring] + # Use a pointer to free the memory in destroyShared + peerAddresses: ptr UncheckedArray[cstring] + peerAddressesLen: int proc createShared*( T: type NodeP2PRequest, @@ -32,11 +34,23 @@ proc createShared*( var ret = createShared(T) ret[].operation = op ret[].peerId = peerId.alloc() - ret[].peerAddresses = peerAddresses + ret[].peerAddressesLen = peerAddresses.len + + if peerAddresses.len > 0: + ret[].peerAddresses = cast[ptr UncheckedArray[cstring]](allocShared( + sizeof(cstring) * peerAddresses.len + )) + for i in 0 ..< peerAddresses.len: + ret[].peerAddresses[i] = peerAddresses[i] + return ret proc destroyShared*(self: ptr NodeP2PRequest) = deallocShared(self[].peerId) + + if self[].peerAddresses != nil: + deallocShared(self[].peerAddresses) + deallocShared(self) proc connect( @@ -88,7 +102,13 @@ proc process*( case self.operation of NodeP2PMsgType.CONNECT: - let res = (await connect(storage, self.peerId, self.peerAddresses)) + # connect() takes seq[cstring]; self.peerAddresses is a raw shared array, + # so copy the pointers into a seq. + var peerAddresses = newSeq[cstring](self.peerAddressesLen) + for i in 0 ..< self.peerAddressesLen: + peerAddresses[i] = self.peerAddresses[i] + + let res = (await connect(storage, self.peerId, peerAddresses)) if res.isErr: error "Failed to CONNECT.", error = res.error return err($res.error) From ddc2cba2c7f83f7cedf5d6c29b49ebdbec0ecc11 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 21:12:06 +0400 Subject: [PATCH 6/9] Fix leak --- library/libstorage.nim | 1 + 1 file changed, 1 insertion(+) diff --git a/library/libstorage.nim b/library/libstorage.nim index 3481e166..5b26b327 100644 --- a/library/libstorage.nim +++ b/library/libstorage.nim @@ -118,6 +118,7 @@ proc storage_new( ).isOkOr: let msg = $error callback(RET_ERR, unsafeAddr msg[0], cast[csize_t](len(msg)), userData) + discard storage_context.destroyStorageContext(ctx) return nil return ctx From 6a1499c05c10a3b3df3ad914e23ea8182f7b4dbd Mon Sep 17 00:00:00 2001 From: Arnaud Date: Sat, 13 Jun 2026 21:37:39 +0400 Subject: [PATCH 7/9] Consistency --- .../storage_thread_requests/requests/node_upload_request.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/storage_thread_requests/requests/node_upload_request.nim b/library/storage_thread_requests/requests/node_upload_request.nim index b2bd94a5..f0a34cf4 100644 --- a/library/storage_thread_requests/requests/node_upload_request.nim +++ b/library/storage_thread_requests/requests/node_upload_request.nim @@ -81,7 +81,7 @@ proc createShared*( ret[].chunkLen = chunkLen ret[].chunkSize = chunkSize - if chunkLen > 0 and chunkData != nil: + if chunkLen > 0: ret[].chunk = allocShared(chunkLen) copyMem(ret[].chunk, chunkData, chunkLen) From 3e70e792af4f216e2aa54d26b38841985658c9ff Mon Sep 17 00:00:00 2001 From: Arnaud Date: Mon, 15 Jun 2026 14:30:52 +0400 Subject: [PATCH 8/9] Update library/storage_thread_requests/requests/node_p2p_request.nim Co-authored-by: Eric <5089238+emizzle@users.noreply.github.com> Signed-off-by: Arnaud --- library/storage_thread_requests/requests/node_p2p_request.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/storage_thread_requests/requests/node_p2p_request.nim b/library/storage_thread_requests/requests/node_p2p_request.nim index 44a45e40..fd19f17f 100644 --- a/library/storage_thread_requests/requests/node_p2p_request.nim +++ b/library/storage_thread_requests/requests/node_p2p_request.nim @@ -48,7 +48,7 @@ proc createShared*( proc destroyShared*(self: ptr NodeP2PRequest) = deallocShared(self[].peerId) - if self[].peerAddresses != nil: + if self[] != nil and self[].peerAddresses != nil: deallocShared(self[].peerAddresses) deallocShared(self) From bba3941c723e347b7cd0cfa6c0a08adb972e4bc3 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Mon, 15 Jun 2026 16:24:40 +0400 Subject: [PATCH 9/9] Fix guard check --- .../storage_thread_requests/requests/node_p2p_request.nim | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/storage_thread_requests/requests/node_p2p_request.nim b/library/storage_thread_requests/requests/node_p2p_request.nim index fd19f17f..91a55b1e 100644 --- a/library/storage_thread_requests/requests/node_p2p_request.nim +++ b/library/storage_thread_requests/requests/node_p2p_request.nim @@ -46,9 +46,12 @@ proc createShared*( return ret proc destroyShared*(self: ptr NodeP2PRequest) = + if self == nil: + return + deallocShared(self[].peerId) - if self[] != nil and self[].peerAddresses != nil: + if self[].peerAddresses != nil: deallocShared(self[].peerAddresses) deallocShared(self)