go-libp2p-pubsub icon indicating copy to clipboard operation
go-libp2p-pubsub copied to clipboard

Unclosed reader in github.com/libp2p/go-libp2p-pubsub.(*PubSub).handleNewStream

Open prestonvanloon opened this issue 6 years ago • 5 comments
trafficstars

This ReaderCloser is never closed. We are seeing high memory usage on the heap from this line and I suspect it is because r.Close() is never called.

https://github.com/libp2p/go-libp2p-pubsub/blob/49274b0e8aecdf6cad59d768e5702ff00aa48488/comm.go#L33

prestonvanloon avatar May 27 '19 15:05 prestonvanloon

I think the closer for this reader is just the closer of the underlying stream, but there are branches which the closer is not called.

Why not add defer r.Close() after opening the reader?

prestonvanloon avatar May 27 '19 15:05 prestonvanloon

@vyzo is looking into this right now!

raulk avatar May 27 '19 17:05 raulk

The stream is properly clsoed or reset in all branches.

There is a test here: https://github.com/libp2p/go-libp2p-pubsub/blob/49274b0e8aecdf6cad59d768e5702ff00aa48488/comm.go#L38 If it is EOF then we can Close write and be done, in the other two cases the stream is Reset.

Note that defer Close() is not sufficient, as we may need to Reset in abnormal termination cases.

vyzo avatar May 27 '19 17:05 vyzo

I think this part of the logic does have problems. One of my services introduced go-ipfs. It runs for a while (sometimes, sometimes two hours) and throws out of menery to exit abnormally.

I don't know why it is like this now, but I suspect it is because stream is never called.

66810012-023e1500-ef61-11e9-8298-1ffc71b27027

profile001

bruinxs avatar Oct 15 '19 07:10 bruinxs

Can you pull a goroutine dump? You should see a tone of handlePeerEOF and/or handleSendingMessages goroutines.

Stebalien avatar Nov 01 '19 17:11 Stebalien