Skip to content

fix: prune empty topic entries from topics map#543

Open
lodekeeper wants to merge 2 commits intoChainSafe:masterfrom
lodekeeper:fix/topic-map-empty-set-prune
Open

fix: prune empty topic entries from topics map#543
lodekeeper wants to merge 2 commits intoChainSafe:masterfrom
lodekeeper:fix/topic-map-empty-set-prune

Conversation

@lodekeeper
Copy link

Summary

Fix a topics map retention bug in gossipsub.

When a remote peer unsubscribed from a topic, or disconnected after being the last subscriber for a topic, the internal topics: Map<TopicStr, Set<PeerIdStr>> entry was left behind as an empty Set. Over time this let topic strings accumulate indefinitely.

This patch:

  • prunes empty topics entries on remote unsubscribe
  • prunes empty topics entries on peer disconnect
  • avoids creating empty topics entries from unsubscribe-only updates
  • adds regression tests for all of the above

Closes #542.

Details

The minimal fix is intentionally scoped to topics only.

I did not prune mesh entries in the same change because mesh is local joined-topic state and deleting empty mesh topics on disconnect would interfere with heartbeat maintenance/regrafting. fanout already has TTL cleanup, and backoff already self-prunes empty per-topic maps.

Testing

  • pnpm test -- --target node --grep 'topic entries|unsubscribe-only'
  • pnpm lint

AI disclosure

Initial heap analysis and patch drafting were assisted by AI tooling, then validated with local source inspection and test runs.

@wemeetagain
Copy link
Member

@lodekeeper please open this PR against the js-libp2p monorepo. This repo will soon be archived

@lodekeeper
Copy link
Author

Reopened this against the js-libp2p monorepo here:\n\n- libp2p/js-libp2p#3405\n\nThis is the same empty-topic pruning fix from this PR, ported under packages/gossipsub in the monorepo per your request.

nflaig pushed a commit to ChainSafe/lodestar that referenced this pull request Mar 13, 2026
## Summary

Open the Lodestar side of the memory-leak mitigation by clearing
composed response-timeout signals in req/resp request handling.

This is the Lodestar-local follow-up for the network-thread leak
investigation in #8969. It addresses the req/resp timeout-signal
retention path by replacing the `AbortSignal.any(...)` composition with
a clearable composed signal and explicitly clearing listeners once the
response stream is done.

This PR is intentionally scoped to the req/resp fix only.
The separate residual gossipsub topic-retention issue is being tracked
upstream in `ChainSafe/js-libp2p-gossipsub#543`, and we can pick that up
once a new libp2p release lands.

## Change

- add `createRespSignal()` helper in
`packages/reqresp/src/request/index.ts`
- compose request signal + timeout signal via an `AbortController`
- remove abort listeners explicitly via `respSignal.clear()` after the
response closes

## Validation

- `pnpm build`
- `pnpm lint` *(warning-only: pre-existing unused biome suppression in
`packages/light-client/test/unit/webEsmBundle.browser.test.ts`)*

## AI disclosure

This PR was authored with AI assistance (Lodekeeper 🌟).

---------

Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com>
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.

Memory leak: prune empty topic entries from gossipsub topics map

2 participants