diff --git a/internal/engine/internal/sessionresolver/childresolver.go b/internal/engine/internal/sessionresolver/childresolver.go index f5897227cf..be9aa69f6c 100644 --- a/internal/engine/internal/sessionresolver/childresolver.go +++ b/internal/engine/internal/sessionresolver/childresolver.go @@ -22,27 +22,16 @@ func timeLimitedLookup(ctx context.Context, re model.Resolver, hostname string) return timeLimitedLookupWithTimeout(ctx, re, hostname, defaultTimeLimitedLookupTimeout) } -// timeLimitedLookupResult is the result of a timeLimitedLookup -type timeLimitedLookupResult struct { - addrs []string - err error -} - // timeLimitedLookupWithTimeout is like timeLimitedLookup but with explicit timeout. func timeLimitedLookupWithTimeout(ctx context.Context, re model.Resolver, hostname string, timeout time.Duration) ([]string, error) { - outch := make(chan *timeLimitedLookupResult, 1) // buffer + // In https://github.com/ooni/probe-cli/pull/807, I modified this code to + // run in a background goroutine and this resulted in a data race, see + // https://github.com/ooni/probe/issues/2135#issuecomment-1149840579. While + // I could not reproduce the data race in a simple way, the race itself + // seems to happen inside the http3 package. For now, I am going to revert + // the change causing the race and I'll investigate later. ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - go func() { - out := &timeLimitedLookupResult{} - out.addrs, out.err = re.LookupHost(ctx, hostname) - outch <- out - }() - select { - case <-ctx.Done(): - return nil, ctx.Err() - case out := <-outch: - return out.addrs, out.err - } + return re.LookupHost(ctx, hostname) } diff --git a/internal/engine/internal/sessionresolver/childresolver_test.go b/internal/engine/internal/sessionresolver/childresolver_test.go index e6a2bb7c3d..e0a7c67370 100644 --- a/internal/engine/internal/sessionresolver/childresolver_test.go +++ b/internal/engine/internal/sessionresolver/childresolver_test.go @@ -5,7 +5,6 @@ import ( "errors" "io" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/model/mocks" @@ -43,25 +42,3 @@ func TestTimeLimitedLookupFailure(t *testing.T) { t.Fatal("expected nil here") } } - -func TestTimeLimitedLookupWillTimeout(t *testing.T) { - done := make(chan bool) - block := make(chan bool) - re := &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - defer close(done) - <-block - return nil, io.EOF - }, - } - ctx := context.Background() - out, err := timeLimitedLookupWithTimeout(ctx, re, "dns.google", 10*time.Millisecond) - if !errors.Is(err, context.DeadlineExceeded) { - t.Fatal("not the error we expected", err) - } - if out != nil { - t.Fatal("expected nil here") - } - close(block) - <-done -}