search

celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
19 stars 16 forks source link

p2p: peerTracker.gc leaks time.Ticker due to missing defer ticker.Stop(); please integrate leak static analyzer #223

Open odeke-em opened 1 month ago

odeke-em commented 1 month ago

https://github.com/celestiaorg/go-header/blob/18f0eb1c5d3aeab6476123da0dad57e82c067984/p2p/peer_tracker.go#L228 leaks the ticker due to a missing defer ticker.Stop() For an efficient p2p mechanism, let’s ensure that we gut out noise and RAM consumers like this. I’d suggest integrating static analyzers in the workflow and CI process. At Orijtech Inc we built a bunch of tools at https://cyber.orijtech.com/tools but for this case we produced tickeryzer https://medium.com/orijtech-developers/tickeryzer-static-analyzer-to-report-missing-time-ticker-stop-call-which-causes-resource-leak-2060a617a830

Kindly cc-ing @Wondertan @walldiss @liamsi

Wondertan commented 1 month ago

True. Luckily, it runs once per node instance for a lifetime of application without any practical leaking.

A simple good first contribution to the lib