Skip to content

fix: resolve query lock bottleneck, relay finder untracked worker, and metrics pool memory leak - #3522

Open
Sahil-4555 wants to merge 9 commits into
libp2p:masterfrom
Sahil-4555:fix/query-relay-pool-fixes
Open

fix: resolve query lock bottleneck, relay finder untracked worker, and metrics pool memory leak#3522
Sahil-4555 wants to merge 9 commits into
libp2p:masterfrom
Sahil-4555:fix/query-relay-pool-fixes

Conversation

@Sahil-4555

@Sahil-4555 Sahil-4555 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

I saw some concurrency, lifecycle and memory management issues in the codebase that can lead to performance bottlenecks, untracked background goroutines and memory retention. I fixed these three problems in this PR.

Here are the issues and what we have done to address them:

1. DHT query events mutex lock contention

  • Issue: In eventChannel.send, the mutex e.mu was held while executing a blocking select statement waiting to send an event into the channel. If the consumer was slow (the channel buffer was full) the write goroutine would block in the select, still holding the mutex. This caused all other concurrent writers/publishers to block immediately on e.mu.Lock(), fully serialising the event publishing path.
  • Fix: Copy the channel reference under lock and immediately release the mutex before entering the select block. To cover the case where waitThenClose closes the channel concurrently, we added a deferred recover() block that catches and safely ignores the closed-channel panic.

2. Found Untracked Goroutine in Relay Finder

  • Bug: The background goroutine go rf.cleanupDisconnectedPeers(ctx) was launched on startup and not registered with rf.refCount (WaitGroup). Consequently, calling Stop() would return before this cleanup loop had fully exited, which could lead to subscription leaks or test flakiness during host shutdown.
  • Fix: Add rf.refCount.Add(1) and deferred Done() the right way, wrapping call in rf.refCount tracking.

3. Metrics Helper Reference Preservation of sync.Pool

  • Issue: Slices pooled in metricshelper were reset with (*s)[:0]. The slice to 0 resets the length, but the elements of the backing array (the string headers) are not touched. This prevents GC to free the referenced strings and the backing byte arrays, resulting in a hidden memory leak/retention.
  • Fix: I added a call to the Go 1.21 clear(*s) builtin inside PutStringSlice before returning the slice pointer to the pool so GC can clean up the memory immediately.

@Sahil-4555 Sahil-4555 changed the title Fix/query relay pool fixes fix: resolve query lock bottleneck, relay finder untracked worker, and metrics pool memory leak Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant