Wait for all go routines to finish before function returns

[hannahhoward]

Goals

Make sure than when EnumerateChildrenAsync returns, all of it's go routines have completed, even in the case of a context cancel

Implementation

Add a waitGroup that waits for all of the graph traversal child go routines to terminate before the function terminates

For Discussion

Reasoning:
See ipfs/go-unixfs#39 (comment)

In the case of a context cancel, the select loop in the main thread of EnumerateChildrenAsyncDepth can receive the cancel before it's processed by the go routines enumerating children. This can cause the EnumerateChildrenAsyncDepth to return with these enumeration routines still in progress, and therefore for getLinks() to get called up to one time per channel after the overall routine returns.

The consequences for the caller of the function can be super confusing, since getLinks is provided by the caller, who make not be expecting getLinks to get called after the function returns. A panic can arise if getLinks attempts to write to a channel the caller has closed after EnumerateChildrenAsyncDepth has returned.

child of ipfs/kubo#5600

[schomatis]

If we are waiting on all goroutines shouldn't getLinks be called with the fetchersCtx context? If there's an error the cancel issued won't make the goroutine(s) inside getLinks return, the whole function will be waiting on that.

This should help to clean up the emitResult function, right?

[schomatis]

Oh, this was already merged, nevermind.

[hannahhoward]

@schomatis we can cleanup emitResult now.

I don't believe the issue you're describing applies. The panic arises from a higher level context cancel(), not fetchersCtx()

[schomatis]

The panic arises from a higher level context cancel(), not fetchersCtx()

What panic? I'm not following, I meant (but I may be off here) that if for whatever reason one of the gorotuines fails (sending a message to errChan) EnumerateChildrenAsyncDepth will try to return but it will be stuck waiting for the rest of the goroutines that might still be inside getLinks, the cancel() being issued won't get them out of there, just out of the loop (which is already terminated by closing feed).