hotfix(sessionresolver): prevent data race inside http3 (#809)
See https://github.com/ooni/probe/issues/2135#issuecomment-1149840579
This commit is contained in:
parent
fe29b432e0
commit
f1b8071c65
|
@ -22,27 +22,16 @@ func timeLimitedLookup(ctx context.Context, re model.Resolver, hostname string)
|
||||||
return timeLimitedLookupWithTimeout(ctx, re, hostname, defaultTimeLimitedLookupTimeout)
|
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.
|
// timeLimitedLookupWithTimeout is like timeLimitedLookup but with explicit timeout.
|
||||||
func timeLimitedLookupWithTimeout(ctx context.Context, re model.Resolver,
|
func timeLimitedLookupWithTimeout(ctx context.Context, re model.Resolver,
|
||||||
hostname string, timeout time.Duration) ([]string, error) {
|
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)
|
ctx, cancel := context.WithTimeout(ctx, timeout)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
go func() {
|
return re.LookupHost(ctx, hostname)
|
||||||
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
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -5,7 +5,6 @@ import (
|
||||||
"errors"
|
"errors"
|
||||||
"io"
|
"io"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
|
||||||
|
|
||||||
"github.com/google/go-cmp/cmp"
|
"github.com/google/go-cmp/cmp"
|
||||||
"github.com/ooni/probe-cli/v3/internal/model/mocks"
|
"github.com/ooni/probe-cli/v3/internal/model/mocks"
|
||||||
|
@ -43,25 +42,3 @@ func TestTimeLimitedLookupFailure(t *testing.T) {
|
||||||
t.Fatal("expected nil here")
|
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
|
|
||||||
}
|
|
||||||
|
|
Loading…
Reference in New Issue
Block a user