libp2p.dialProtocol promises are retained in memory

[twoeths]

Describe the bug

This is from a heap snapshot I use to sync Pyrmont

Expected behavior

• The stream and Promise are not retained in memory
• finalizeRoot should not carry the Tree object

Steps to Reproduce

• Sync Pyrmont

[twoeths]

this is another view where the retained tree stay in finalizedRoot of Status message

All retained trees in Status message takes ~200MB

[twoeths]

so the libp2p.dialProtocol promises were retained in memory, and it keeps the stream with it while the stream.sink keeps the requestEncode generator along with finalizeRoot (and a tree)

and the retained libp2p.dialProtocol promises happen with other protocols as well (beacon_blocks_by_range and ipfs etc.), not just status.

[dapplion]

In regards to the finalizedRoot:

When accessing a property of the state, a new SubTree is created which holds a reference to the parent tree (the entire state). For example here:

lodestar/packages/lodestar-fork-choice/src/forkChoice/forkChoice.ts

Line 293 in 867db03

this.fcStore.finalizedCheckpoint = state.finalizedCheckpoint;

ssz getter code: https://github.com/ChainSafe/ssz/blob/d6efe104a3bdc2d2beebc47af8234a3e40d414f4/src/backings/tree/container.ts#L142

getProperty<V extends keyof T>(target: Tree, property: V): PropOfTreeBacked<T, V> {
   ...
      return fieldType.tree.asTreeBacked(this.getSubtreeAtChunk(target, chunkIndex)) as PropOfTreeBacked<T, V>;
  }

persistent-merkle-tree code:
https://github.com/ChainSafe/persistent-merkle-tree/blob/44ec217a1531137ef9d9ebdd2fdf653559e3d4d6/src/tree.ts#L83

getSubtree(index: Gindex): Tree {
    return new Tree(this.getNode(index), (v: Tree): void => this.setNode(index, v.rootNode));
  }

So if someone grabs finalizedCheckpoint from the ForkChoice and keeps that for long the entire state won't ever be garbage collected.

• Is this expected? This behavior can cause un-expected memory leaks very hard to trace since random variables could be holding to big state without being immediately obvious.
• Could finalizedCheckpoint be cloned to break that reference?
• Could the automatic binding of subTrees be configurable somehow? How often we need a binded subtree vs how often don't?

[dapplion]

Proposed solution ChainSafe/ssz#83 (comment)

[dapplion]

This seems to be fixed completely with ChainSafe/persistent-merkle-tree#26