Skip to content
This repository was archived by the owner on Jul 22, 2026. It is now read-only.

fix: prune empty topic entries from topics map - #543

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

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

Conversation

@lodekeeper

Copy link
Copy Markdown

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
Copy Markdown
Member

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

@lodekeeper

Copy link
Copy Markdown
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>

This branch has not been deployed

No deployments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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