Potential goroutine leak in storage/cache/subtree_cache.go (method SubtreeCache.preload())

[nicolasdilley]

I think there is a potential goroutine leak in method preload() at lines 108-187 of storage/cache/subtree_cache.go.

For instance, assume:

• len(want) = 1
• s.populateConcurrency = 3
• len(subtrees) = 3

If err != nil at line 170 in the first iteration of the range, then method preload() will return while three goroutines are stuck:

• Two goroutines are stuck at line 159 waiting to send on channel ch which is full.
• One goroutine is stuck at line 164 waiting on wg (which needs the two goroutines above to finish).

A potential fix is to change line 141
ch := make(chan *storagepb.SubtreeProto, len(want)) to
ch := make(chan *storagepb.SubtreeProto, int(math.Max(len(want), len(subtrees)))

FYI, I have found this bug with my tool Gomela which is a static analyser for concurrent Go programs. Let me know if you find this useful!

[pav-kv]

Hi Nicolas,

Thanks for putting your time in this. I'm looking at it right now, and will fix ASAP.

[pav-kv]

The described scenario shouldn't happen because:

• len(subtrees) <= len(want) (the getSubtrees function will not return more subtrees than requested)
• therefore, there won't be blockage in line 159

However, I think we can add an assert that len(subtrees) <= len(want) actually holds, before starting the goroutines.

[pav-kv]

Thanks Nicolas. Please let us know / reopen this issue if your tool still alerts.

[nicolasdilley]

No problem!
Yes this would be a good addition since getSubstrees is given as a parameter.
Ok I will keep you posted if my tool still reports a goroutine leak!