From 7348fb9edb68ca9142f5d87673da48cef733b3d3 Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sun, 5 Jun 2022 15:01:08 +0200 Subject: github: fix data race when closing event channel I believe the issue was twofold: When done importing, the calling context is likely still valid, so if the output channel is not read enough and reach capacity, some event producer down the line can be blocked trying to send in that channel. When closing it, this send is still trying to proceed, which is illegal in go. In rateLimitHandlerClient, there was a need to 2 different type of output channel: core.ExportResult and ImportEvent. To do so, the previous code was using a single channel type RateLimitingEvent and a series of goroutines to read/cast/send to the final channel. This could result in more async goroutine being stuck trying to send in an at-capacity channel. Instead, the code now use a simple synchronous callback to directly push to the final output channel. No concurrency needed anymore and the code is simpler. Any of those fixes could have resolved the data race, but both fixes is more correct. --- bridge/github/export.go | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) (limited to 'bridge/github/export.go') diff --git a/bridge/github/export.go b/bridge/github/export.go index 8c40eb74..35d456c2 100644 --- a/bridge/github/export.go +++ b/bridge/github/export.go @@ -486,23 +486,10 @@ func (ge *githubExporter) cacheGithubLabels(ctx context.Context, gc *rateLimitHa } q := labelsQuery{} - // When performing the queries we have to forward rate limiting events to the - // current channel of export results. - events := make(chan RateLimitingEvent) - defer close(events) - go func() { - for e := range events { - select { - case <-ctx.Done(): - return - case ge.out <- core.NewExportRateLimiting(e.msg): - } - } - }() hasNextPage := true for hasNextPage { - if err := gc.queryWithLimitEvents(ctx, &q, variables, events); err != nil { + if err := gc.queryExport(ctx, &q, variables, ge.out); err != nil { return err } -- cgit