Skip to content

Commit

Permalink
fix(ptx/obfs4_test.go): avoid context-vs-normal-code race
Browse files Browse the repository at this point in the history
We want to test whether we get the context failure if the error
generated inside normal code happens _after_ the context cancellation.

The best way to do that is to write code that is not racy. To this
end, we just need to pause normal code until we know that the context
has returned to the caller. We also need to ensure we do not leak
a goroutine, hence we use a WaitGroup to check that.

Fixes ooni/probe#1750
  • Loading branch information
bassosimone committed Sep 5, 2021
1 parent b2085fe commit d0a41dd
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions internal/ptx/obfs4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"net"
"strings"
"sync"
"testing"

"github.com/ooni/probe-cli/v3/internal/atomicx"
Expand Down Expand Up @@ -65,14 +66,17 @@ func TestOBFS4DialerFailsWithConnectionErrorAndNoContextExpiration(t *testing.T)
func TestOBFS4DialerFailsWithConnectionErrorAndContextExpiration(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
expected := errors.New("mocked error")
unexpected := errors.New("mocked error")
o4d := DefaultTestingOBFS4Bridge()
sigch := make(chan interface{})
wg := &sync.WaitGroup{}
wg.Add(1)
o4d.UnderlyingDialer = &netxmocks.Dialer{
MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) {
// We cancel the context before returning the error, which makes
// the context cancellation happen before us returning.
cancel()
return nil, expected
<-sigch
wg.Done()
return nil, unexpected
},
}
conn, err := o4d.DialContext(ctx)
Expand All @@ -82,6 +86,8 @@ func TestOBFS4DialerFailsWithConnectionErrorAndContextExpiration(t *testing.T) {
if conn != nil {
t.Fatal("expected nil conn here")
}
close(sigch)
wg.Wait()
}

// obfs4connwrapper allows us to observe that Close has been called
Expand Down

0 comments on commit d0a41dd

Please sign in to comment.