retrieval: fix memory leak

[acud]

We have a memory leak on production and it seems that this is the problem:

1. We issue a retrieve request to a peer, adding an entry to retrievals map on the retrieval peer in addRetrieval
2. Peer cannot find the chunk, never returning an error
3. Netstore RemoteFetch times out on context or defined search timeout
4. Entry on Peer never gets deleted
5. Due to the fact that the peers is stable and not disconnected - the map on the Peer struct leaks

It is not possible to create a regression test on netstore due to circular dependencies.

EDIT: I have added a test that checks that NoSuitablePeer error is emitted correctly from netstore. To verify that the leak happens, modify expireRetrieval to the following, then run TestNoSuitablePeer test. it will panic because the record is found in memory, exacerbating the problem.

func (p *Peer) expireRetrieval(ruid uint) {
	p.mtx.Lock()
	defer p.mtx.Unlock()

	if _, found := p.retrievals[ruid]; found {
		panic("found")
	}

	delete(p.retrievals, ruid)
}

[janos]

It would be good to have a short explanation what is the purpose of the returned function, as now, it requires to go through the code. Or to name it in function signature.

[janos]

Is it possible that if this sleep is too short, the test passes but from different reason then this test is designed for? It would not have any peers in kademlia. Should there be some sort of check to see if peers are connected?

[acud]

it would fail in both cases. i added a check nevertheless

[janos]

Thanks. Unfortunately, now it's timing out https://travis-ci.org/ethersphere/swarm/jobs/652344397#L1236.

[acud]

pushed a fix

[pradovic]

👍