feat(enginenetx): implement deterministic+random mixing (#1558)

We need deterministic+random mixing of HTTPS dial tactics to ensure that
we prioritize some tactics coming from the DNS before attempting all the
previous tactics, which would make the bootstrap super slow.

Part of https://github.com/ooni/probe/issues/2704.

internal/enginenetx/bridgespolicy.go

@@ -27,33 +27,18 @@ var _ httpsDialerPolicy = &bridgesPolicy{}
 
 // LookupTactics implements httpsDialerPolicy.
 func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
-	out := make(chan *httpsDialerTactic)
-
-	go func() {
-		defer close(out) // tell the parent when we're done
-		index := 0
-
+	return mixSequentially(
 		// emit bridges related tactics first which are empty if there are
 		// no bridges for the givend domain and port
-		for tx := range p.bridgesTacticsForDomain(domain, port) {
-			tx.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- tx
-		}
+		p.bridgesTacticsForDomain(domain, port),
 
 		// now fallback to get more tactics (typically here the fallback
 		// uses the DNS and obtains some extra tactics)
 		//
 		// we wrap whatever the underlying policy returns us with some
 		// extra logic for better communicating with test helpers
-		for tx := range p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)) {
-			tx.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- tx
-		}
-	}()
-
-	return out
+		p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)),
+	)
 }
 
 var bridgesPolicyTestHelpersDomains = []string{
@@ -92,7 +77,7 @@ func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialer
 			for _, sni := range p.bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        tactic.Address,
-					InitialDelay:   0,
+					InitialDelay:   0, // set when dialing
 					Port:           tactic.Port,
 					SNI:            sni,
 					VerifyHostname: tactic.VerifyHostname,
@@ -119,7 +104,7 @@ func (p *bridgesPolicy) bridgesTacticsForDomain(domain, port string) <-chan *htt
 			for _, sni := range p.bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        ipAddr,
-					InitialDelay:   0,
+					InitialDelay:   0, // set when dialing
 					Port:           port,
 					SNI:            sni,
 					VerifyHostname: domain,

internal/enginenetx/bridgespolicy_test.go

@@ -9,7 +9,7 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/model"
 )
 
-func TestBeaconsPolicy(t *testing.T) {
+func TestBridgesPolicy(t *testing.T) {
 	t.Run("for domains for which we don't have bridges and DNS failure", func(t *testing.T) {
 		expected := errors.New("mocked error")
 		p := &bridgesPolicy{
@@ -62,6 +62,10 @@ func TestBeaconsPolicy(t *testing.T) {
 				t.Fatal("the host should always be 93.184.216.34")
 			}
 
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
 			if tactic.SNI != "www.example.com" {
 				t.Fatal("the SNI field should always be like `www.example.com`")
 			}
@@ -76,7 +80,7 @@ func TestBeaconsPolicy(t *testing.T) {
 		}
 	})
 
-	t.Run("for the api.ooni.io domain", func(t *testing.T) {
+	t.Run("for the api.ooni.io domain with DNS failure", func(t *testing.T) {
 		expected := errors.New("mocked error")
 		p := &bridgesPolicy{
 			Fallback: &dnsPolicy{
@@ -92,6 +96,7 @@ func TestBeaconsPolicy(t *testing.T) {
 		ctx := context.Background()
 		tactics := p.LookupTactics(ctx, "api.ooni.io", "443")
 
+		// since the DNS fails, we should only see tactics generated by bridges
 		var count int
 		for tactic := range tactics {
 			count++
@@ -103,6 +108,10 @@ func TestBeaconsPolicy(t *testing.T) {
 				t.Fatal("the host should always be 162.55.247.208")
 			}
 
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
 			if tactic.SNI == "api.ooni.io" {
 				t.Fatal("we should not see the `api.ooni.io` SNI on the wire")
 			}
@@ -117,6 +126,81 @@ func TestBeaconsPolicy(t *testing.T) {
 		}
 	})
 
+	t.Run("for the api.ooni.io domain with DNS success", func(t *testing.T) {
+		p := &bridgesPolicy{
+			Fallback: &dnsPolicy{
+				Logger: model.DiscardLogger,
+				Resolver: &mocks.Resolver{
+					MockLookupHost: func(ctx context.Context, domain string) ([]string, error) {
+						return []string{"130.192.91.211"}, nil
+					},
+				},
+			},
+		}
+
+		ctx := context.Background()
+		tactics := p.LookupTactics(ctx, "api.ooni.io", "443")
+
+		// since the DNS succeeds we should see bridge tactics mixed with DNS tactics
+		var (
+			bridgesCount int
+			dnsCount     int
+			overallCount int
+		)
+		const expectedDNSEntryCount = 153 // yikes!
+		for tactic := range tactics {
+			overallCount++
+
+			t.Log(overallCount, tactic)
+
+			if tactic.Port != "443" {
+				t.Fatal("the port should always be 443")
+			}
+
+			switch {
+			case overallCount == expectedDNSEntryCount:
+				if tactic.Address != "130.192.91.211" {
+					t.Fatal("the host should be 130.192.91.211 for count ==", expectedDNSEntryCount)
+				}
+
+				if tactic.SNI != "api.ooni.io" {
+					t.Fatal("we should see the `api.ooni.io` SNI on the wire for count ==", expectedDNSEntryCount)
+				}
+
+				dnsCount++
+
+			default:
+				if tactic.Address != "162.55.247.208" {
+					t.Fatal("the host should be 162.55.247.208 for count !=", expectedDNSEntryCount)
+				}
+
+				if tactic.SNI == "api.ooni.io" {
+					t.Fatal("we should not see the `api.ooni.io` SNI on the wire for count !=", expectedDNSEntryCount)
+				}
+
+				bridgesCount++
+			}
+
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
+			if tactic.VerifyHostname != "api.ooni.io" {
+				t.Fatal("the VerifyHostname field should always be like `api.ooni.io`")
+			}
+		}
+
+		if overallCount <= 0 {
+			t.Fatal("expected to see at least one tactic")
+		}
+		if dnsCount != 1 {
+			t.Fatal("expected to see exactly one DNS based tactic")
+		}
+		if bridgesCount <= 0 {
+			t.Fatal("expected to see at least one bridge tactic")
+		}
+	})
+
 	t.Run("for test helper domains", func(t *testing.T) {
 		for _, domain := range bridgesPolicyTestHelpersDomains {
 			t.Run(domain, func(t *testing.T) {
@@ -134,27 +218,25 @@ func TestBeaconsPolicy(t *testing.T) {
 				}
 
 				ctx := context.Background()
-				index := 0
-				for tactics := range p.LookupTactics(ctx, domain, "443") {
+				for tactic := range p.LookupTactics(ctx, domain, "443") {
 
-					if tactics.Address != "164.92.180.7" {
+					if tactic.Address != "164.92.180.7" {
 						t.Fatal("unexpected .Address")
 					}
 
-					if tactics.InitialDelay != happyEyeballsDelay(index) {
+					if tactic.InitialDelay != 0 {
 						t.Fatal("unexpected .InitialDelay")
 					}
-					index++
 
-					if tactics.Port != "443" {
+					if tactic.Port != "443" {
 						t.Fatal("unexpected .Port")
 					}
 
-					if tactics.SNI == domain {
+					if tactic.SNI == domain {
 						t.Fatal("unexpected .Domain")
 					}
 
-					if tactics.VerifyHostname != domain {
+					if tactic.VerifyHostname != domain {
 						t.Fatal("unexpected .VerifyHostname")
 					}
 				}

internal/enginenetx/dnspolicy.go

@@ -56,10 +56,10 @@ func (p *dnsPolicy) LookupTactics(
 		}
 
 		// The tactics we generate here have SNI == VerifyHostname == domain
-		for idx, addr := range addrs {
+		for _, addr := range addrs {
 			tactic := &httpsDialerTactic{
 				Address:        addr,
-				InitialDelay:   happyEyeballsDelay(idx),
+				InitialDelay:   0, // set when dialing
 				Port:           port,
 				SNI:            domain,
 				VerifyHostname: domain,

internal/enginenetx/dnspolicy_test.go

@@ -54,6 +54,9 @@ func TestDNSPolicy(t *testing.T) {
 			if tactic.Address != "130.192.91.211" {
 				t.Fatal("invalid endpoint address")
 			}
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected .InitialDelay")
+			}
 			if tactic.Port != "443" {
 				t.Fatal("invalid endpoint port")
 			}

internal/enginenetx/filter.go

@@ -0,0 +1,73 @@
+package enginenetx
+
+// filterOutNilTactics filters out nil tactics.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func filterOutNilTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		for tx := range input {
+			if tx != nil {
+				output <- tx
+			}
+		}
+	}()
+	return output
+}
+
+// filterOnlyKeepUniqueTactics only keeps unique tactics.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func filterOnlyKeepUniqueTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+
+		// make sure we close output chan
+		defer close(output)
+
+		// useful to make sure we don't emit two equal policy in a single run
+		uniq := make(map[string]int)
+
+		for tx := range input {
+			// handle the case in which we already emitted a tactic
+			key := tx.tacticSummaryKey()
+			if uniq[key] > 0 {
+				continue
+			}
+			uniq[key]++
+
+			// emit the tactic
+			output <- tx
+		}
+
+	}()
+	return output
+}
+
+// filterAssignInitialDelays assigns initial delays to tactics.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func filterAssignInitialDelays(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+
+		// make sure we close output chan
+		defer close(output)
+
+		index := 0
+		for tx := range input {
+			// rewrite the delays
+			tx.InitialDelay = happyEyeballsDelay(index)
+			index++
+
+			// emit the tactic
+			output <- tx
+		}
+
+	}()
+	return output
+}

internal/enginenetx/filter_test.go

@@ -0,0 +1,111 @@
+package enginenetx
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestFilterOutNilTactics(t *testing.T) {
+	inputs := []*httpsDialerTactic{
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+	}
+
+	expect := []*httpsDialerTactic{
+		inputs[2], inputs[4],
+	}
+
+	var output []*httpsDialerTactic
+	for tx := range filterOutNilTactics(streamTacticsFromSlice(inputs)) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestFilterOnlyKeepUniqueTactics(t *testing.T) {
+	templates := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "www.example.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "www.kernel.org",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "x.org",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "www.polito.it",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	inputs := []*httpsDialerTactic{
+		templates[2], templates[1], templates[1],
+		templates[2], templates[2], templates[1],
+		templates[0], templates[1], templates[0],
+		templates[2], templates[1], templates[2],
+		templates[1], templates[0], templates[1],
+		templates[3], // only once at the end
+	}
+
+	expect := []*httpsDialerTactic{
+		templates[2], templates[1], templates[0], templates[3],
+	}
+
+	var output []*httpsDialerTactic
+	for tx := range filterOnlyKeepUniqueTactics(streamTacticsFromSlice(inputs)) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestFilterAssignInitalDelays(t *testing.T) {
+	inputs := []*httpsDialerTactic{}
+	ff := &testingx.FakeFiller{}
+	ff.Fill(&inputs)
+	idx := 0
+	for tx := range filterAssignInitialDelays(streamTacticsFromSlice(inputs)) {
+		if tx.InitialDelay != happyEyeballsDelay(idx) {
+			t.Fatal("unexpected .InitialDelay", tx.InitialDelay, "for", idx)
+		}
+		idx++
+	}
+	if idx < 1 {
+		t.Fatal("expected to see at least one entry")
+	}
+}

internal/enginenetx/httpsdialer.go

@@ -97,7 +97,7 @@ type httpsDialerPolicy interface {
 
 // httpsDialerEventsHandler handles events occurring while we try dialing TLS.
 type httpsDialerEventsHandler interface {
-	// These callbacks are invoked during the TLS handshake to inform this
+	// These callbacks are invoked during the TLS dialing to inform this
 	// interface about events that occurred. A policy SHOULD keep track of which
 	// addresses, SNIs, etc. work and return them more frequently.
 	//
@@ -209,7 +209,7 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 
 	// The emitter will emit tactics and then close the channel when done. We spawn 16 workers
 	// that handle tactics in parallel and post results on the collector channel.
-	emitter := hd.policy.LookupTactics(ctx, hostname, port)
+	emitter := httpsDialerFilterTactics(hd.policy.LookupTactics(ctx, hostname, port))
 	collector := make(chan *httpsDialerErrorOrConn)
 	joiner := make(chan any)
 	const parallelism = 16
@@ -236,8 +236,10 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 				continue
 			}
 
-			// Save the conn and tell goroutines to stop ASAP
+			// Save the conn
 			connv = append(connv, result.Conn)
+
+			// Interrupt other concurrent dialing attempts
 			cancel()
 		}
 	}
@@ -245,6 +247,20 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 	return httpsDialerReduceResult(connv, errorv)
 }
 
+// httpsDialerFilterTactics filters the tactics to:
+//
+// 1. be paranoid and filter out nil tactics if any;
+//
+// 2. avoid emitting duplicate tactics as part of the same run;
+//
+// 3. rewrite the happy eyeball delays.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func httpsDialerFilterTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	return filterAssignInitialDelays(filterOnlyKeepUniqueTactics(filterOutNilTactics(input)))
+}
+
 // httpsDialerReduceResult returns either an established conn or an error, using [errDNSNoAnswer] in
 // case the list of connections and the list of errors are empty.
 func httpsDialerReduceResult(connv []model.TLSConn, errorv []error) (model.TLSConn, error) {

internal/enginenetx/httpsdialer_test.go

@@ -632,3 +632,231 @@ func TestHTTPSDialerReduceResult(t *testing.T) {
 		}
 	})
 }
+
+// Make sure that (1) we remove nils; (2) we avoid emitting duplicate tactics; (3) we fill
+// the happy-eyeballs delays for each entry we return.
+func TestHTTPSDialerFilterTactics(t *testing.T) {
+	// define the inputs vector including duplicates and nils
+	inputs := []*httpsDialerTactic{
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.com",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "freebsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "dragonflybsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "netbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+		nil,
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		nil,
+	}
+
+	// define the expectations
+	expect := []*httpsDialerTactic{
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   0,
+			Port:           "443",
+			SNI:            "x.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   time.Second,
+			Port:           "443",
+			SNI:            "www.polito.it",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   2 * time.Second,
+			Port:           "443",
+			SNI:            "x.com",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   4 * time.Second,
+			Port:           "443",
+			SNI:            "kerneltrap.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   8 * time.Second,
+			Port:           "443",
+			SNI:            "freebsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   16 * time.Second,
+			Port:           "443",
+			SNI:            "dragonflybsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.211",
+			InitialDelay:   24 * time.Second,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   32 * time.Second,
+			Port:           "443",
+			SNI:            "openbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+		{
+			Address:        "130.192.91.231",
+			InitialDelay:   40 * time.Second,
+			Port:           "443",
+			SNI:            "netbsd.org",
+			VerifyHostname: "api.ooni.io",
+		},
+	}
+
+	// run the algorithm
+	var results []*httpsDialerTactic
+	for tx := range httpsDialerFilterTactics(streamTacticsFromSlice(inputs)) {
+		results = append(results, tx)
+	}
+
+	// compare the results
+	if diff := cmp.Diff(expect, results); diff != "" {
+		t.Fatal(diff)
+	}
+}

internal/enginenetx/mix.go

@@ -0,0 +1,87 @@
+package enginenetx
+
+import "sync"
+
+// mixSequentially mixes entries from primary followed by entries from fallback.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func mixSequentially(primary, fallback <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		for tx := range primary {
+			output <- tx
+		}
+		for tx := range fallback {
+			output <- tx
+		}
+	}()
+	return output
+}
+
+// mixDeterministicThenRandomConfig contains config for [mixDeterministicThenRandom].
+type mixDeterministicThenRandomConfig struct {
+	// C is the channel to mix from.
+	C <-chan *httpsDialerTactic
+
+	// N is the number of entries to read from at the
+	// beginning before starting random mixing.
+	N int
+}
+
+// mixDeterministicThenRandom reads the first N entries from primary, if any, then the first N
+// entries from fallback, if any, and then randomly mixes the entries.
+func mixDeterministicThenRandom(primary, fallback *mixDeterministicThenRandomConfig) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		mixTryEmitN(primary.C, primary.N, output)
+		mixTryEmitN(fallback.C, fallback.N, output)
+		for tx := range mixRandomly(primary.C, fallback.C) {
+			output <- tx
+		}
+	}()
+	return output
+}
+
+func mixTryEmitN(input <-chan *httpsDialerTactic, numToRead int, output chan<- *httpsDialerTactic) {
+	for idx := 0; idx < numToRead; idx++ {
+		tactic, good := <-input
+		if !good {
+			return
+		}
+		output <- tactic
+	}
+}
+
+func mixRandomly(left, right <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		// read from left
+		waitg := &sync.WaitGroup{}
+		waitg.Add(1)
+		go func() {
+			defer waitg.Done()
+			for tx := range left {
+				output <- tx
+			}
+		}()
+
+		// read from right
+		waitg.Add(1)
+		go func() {
+			defer waitg.Done()
+			for tx := range right {
+				output <- tx
+			}
+		}()
+
+		// close when done
+		go func() {
+			waitg.Wait()
+			close(output)
+		}()
+	}()
+	return output
+}

internal/enginenetx/mix_test.go

@@ -0,0 +1,227 @@
+package enginenetx
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestMixSequentially(t *testing.T) {
+	primary := []*httpsDialerTactic{}
+	fallback := []*httpsDialerTactic{}
+
+	ff := &testingx.FakeFiller{}
+	ff.Fill(&primary)
+	ff.Fill(&fallback)
+
+	expect := append([]*httpsDialerTactic{}, primary...)
+	expect = append(expect, fallback...)
+
+	var output []*httpsDialerTactic
+	for tx := range mixSequentially(streamTacticsFromSlice(primary), streamTacticsFromSlice(fallback)) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(expect, output); diff != "" {
+		t.Fatal(diff)
+	}
+}
+
+func TestMixDeterministicThenRandom(t *testing.T) {
+	// define primary data source
+	primary := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a3.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a4.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a5.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a6.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a7.com",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	// define fallback data source
+	fallback := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b3.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b4.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b5.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b6.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b7.com",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	// define the expectations for the beginning of the result
+	expectBeginning := []*httpsDialerTactic{{
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "a2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b1.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b2.com",
+		VerifyHostname: "api.ooni.io",
+	}, {
+		Address:        "130.192.91.211",
+		InitialDelay:   0,
+		Port:           "443",
+		SNI:            "b3.com",
+		VerifyHostname: "api.ooni.io",
+	}}
+
+	// remix
+	outch := mixDeterministicThenRandom(
+		&mixDeterministicThenRandomConfig{
+			C: streamTacticsFromSlice(primary),
+			N: 2,
+		},
+		&mixDeterministicThenRandomConfig{
+			C: streamTacticsFromSlice(fallback),
+			N: 3,
+		},
+	)
+	var output []*httpsDialerTactic
+	for tx := range outch {
+		output = append(output, tx)
+	}
+
+	// make sure we have the expected number of entries
+	if len(output) != 14 {
+		t.Fatal("we need 14 entries")
+	}
+	if diff := cmp.Diff(expectBeginning, output[:5]); diff != "" {
+		t.Fatal(diff)
+	}
+
+	// make sure each entry is represented
+	const (
+		inprimary = 1 << 0
+		infallback
+		inoutput
+	)
+	mapping := make(map[string]int)
+	for _, entry := range primary {
+		mapping[entry.tacticSummaryKey()] |= inprimary
+	}
+	for _, entry := range fallback {
+		mapping[entry.tacticSummaryKey()] |= infallback
+	}
+	for _, entry := range output {
+		mapping[entry.tacticSummaryKey()] |= inoutput
+	}
+	for entry, flags := range mapping {
+		if flags != (inprimary|inoutput) && flags != (infallback|inoutput) {
+			t.Fatal("unexpected flags", flags, "for entry", entry)
+		}
+	}
+}
+
+func TestMixTryEmitNWithClosedChannel(t *testing.T) {
+	// create an already closed channel
+	inputch := make(chan *httpsDialerTactic)
+	close(inputch)
+
+	// create channel for collecting the results
+	outputch := make(chan *httpsDialerTactic)
+
+	go func() {
+		// Implementation note: mixTryEmitN does not close the channel
+		// when done, therefore we need to close it ourselves.
+		mixTryEmitN(inputch, 10, outputch)
+		close(outputch)
+	}()
+
+	// read the output channel
+	var output []*httpsDialerTactic
+	for tx := range outputch {
+		output = append(output, tx)
+	}
+
+	// make sure we didn't read anything
+	if len(output) != 0 {
+		t.Fatal("expected zero entries")
+	}
+}

internal/enginenetx/network.go

@@ -93,7 +93,8 @@ func NewNetwork(
 	netx := &netxlite.Netx{}
 	dialer := netx.NewDialerWithResolver(logger, resolver)
 
-	// Create manager for keeping track of statistics
+	// Create manager for keeping track of statistics. This implies creating a background
+	// goroutine that we'll need to close when we're done.
 	const trimInterval = 30 * time.Second
 	stats := newStatsManager(kvStore, logger, trimInterval)
 
@@ -118,15 +119,8 @@ func NewNetwork(
 	// the proxy, otherwise it means that we're using the ooni/oohttp library
 	// to dial for proxies, which has some restrictions.
 	//
-	// In particular, the returned transport uses dialer for dialing with
-	// cleartext proxies (e.g., socks5 and http) and httpsDialer for dialing
-	// with encrypted proxies (e.g., https). After this has happened,
-	// the code currently falls back to using the standard library's tls
-	// client code for establishing TLS connections over the proxy. The main
-	// implication here is that we're not using our custom mozilla CA for
-	// validating TLS certificates, rather we're using the system's cert store.
-	//
-	// Fixing this issue is TODO(https://github.com/ooni/probe/issues/2536).
+	// - this code does not work as intended when using netem and proxies
+	// as documented by TODO(https://github.com/ooni/probe/issues/2536).
 	txp := netxlite.NewHTTPTransportWithOptions(
 		logger, dialer, httpsDialer,
 		netxlite.HTTPTransportOptionDisableCompression(false),

internal/enginenetx/statsmanager.go

@@ -137,6 +137,8 @@ func statsDefensivelySortTacticsByDescendingSuccessRateWithAcceptPredicate(
 	input []*statsTactic, acceptfunc func(*statsTactic) bool) []*statsTactic {
 	// first let's create a working list such that we don't modify
 	// the input in place thus avoiding any data race
+	//
+	// make sure we explicitly filter out malformed entries
 	work := []*statsTactic{}
 	for _, t := range input {
 		if t != nil && t.Tactic != nil {
@@ -193,8 +195,8 @@ func (st *statsTactic) Clone() *statsTactic {
 	// a pointer to a location which is typically immutable, so it's perfectly
 	// fine to copy the LastUpdate field by assignment.
 	//
-	// here we're using a bunch of robustness aware mechanisms to clone
-	// considering that the struct may be edited by the user
+	// here we're using safe functions to clone the original struct considering
+	// that a user can edit the content on disk freely introducing nulls.
 	return &statsTactic{
 		CountStarted:               st.CountStarted,
 		CountTCPConnectError:       st.CountTCPConnectError,

internal/enginenetx/statspolicy.go

@@ -30,51 +30,17 @@ var _ httpsDialerPolicy = &statsPolicy{}
 
 // LookupTactics implements HTTPSDialerPolicy.
 func (p *statsPolicy) LookupTactics(ctx context.Context, domain string, port string) <-chan *httpsDialerTactic {
-	out := make(chan *httpsDialerTactic)
-
-	go func() {
-		defer close(out) // make sure the parent knows when we're done
-		index := 0
-
-		// useful to make sure we don't emit two equal policy in a single run
-		uniq := make(map[string]int)
-
-		// function that emits a given tactic unless we already emitted it
-		maybeEmitTactic := func(t *httpsDialerTactic) {
-			// as a safety mechanism let's gracefully handle the
-			// case in which the tactic is nil
-			if t == nil {
-				return
-			}
-
-			// handle the case in which we already emitted a policy
-			key := t.tacticSummaryKey()
-			if uniq[key] > 0 {
-				return
-			}
-			uniq[key]++
-
-			// 🚀!!!
-			t.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- t
-		}
-
+	// avoid emitting nil tactics and duplicate tactics
+	return filterOnlyKeepUniqueTactics(filterOutNilTactics(mixSequentially(
 		// give priority to what we know from stats
-		for _, t := range statsPolicyPostProcessTactics(p.Stats.LookupTactics(domain, port)) {
-			maybeEmitTactic(t)
-		}
+		streamTacticsFromSlice(statsPolicyFilterStatsTactics(p.Stats.LookupTactics(domain, port))),
 
 		// fallback to the secondary policy
-		for t := range p.Fallback.LookupTactics(ctx, domain, port) {
-			maybeEmitTactic(t)
-		}
-	}()
-
-	return out
+		p.Fallback.LookupTactics(ctx, domain, port),
+	)))
 }
 
-func statsPolicyPostProcessTactics(tactics []*statsTactic, good bool) (out []*httpsDialerTactic) {
+func statsPolicyFilterStatsTactics(tactics []*statsTactic, good bool) (out []*httpsDialerTactic) {
 	// when good is false, it means p.Stats.LookupTactics failed
 	if !good {
 		return

internal/enginenetx/statspolicy_test.go

@@ -169,21 +169,19 @@ func TestStatsPolicyWorkingAsIntended(t *testing.T) {
 
 		// compute the list of results we expect to see from the stats data
 		var expect []*httpsDialerTactic
-		idx := 0
 		for _, entry := range expectTacticsStats {
 			if entry.CountSuccess <= 0 || entry.Tactic == nil {
 				continue // we SHOULD NOT include entries that systematically failed
 			}
 			t := entry.Tactic.Clone()
-			t.InitialDelay = happyEyeballsDelay(idx)
+			t.InitialDelay = 0
 			expect = append(expect, t)
-			idx++
 		}
 
 		// extend the expected list to include DNS results
 		expect = append(expect, &httpsDialerTactic{
 			Address:        bridgeAddress,
-			InitialDelay:   2 * time.Second,
+			InitialDelay:   0,
 			Port:           "443",
 			SNI:            "api.ooni.io",
 			VerifyHostname: "api.ooni.io",
@@ -234,21 +232,19 @@ func TestStatsPolicyWorkingAsIntended(t *testing.T) {
 
 		// compute the list of results we expect to see from the stats data
 		var expect []*httpsDialerTactic
-		idx := 0
 		for _, entry := range expectTacticsStats {
 			if entry.CountSuccess <= 0 || entry.Tactic == nil {
 				continue // we SHOULD NOT include entries that systematically failed
 			}
 			t := entry.Tactic.Clone()
-			t.InitialDelay = happyEyeballsDelay(idx)
+			t.InitialDelay = 0
 			expect = append(expect, t)
-			idx++
 		}
 
 		// extend the expected list to include DNS results
 		expect = append(expect, &httpsDialerTactic{
 			Address:        bridgeAddress,
-			InitialDelay:   2 * time.Second,
+			InitialDelay:   0,
 			Port:           "443",
 			SNI:            "api.ooni.io",
 			VerifyHostname: "api.ooni.io",
@@ -290,15 +286,13 @@ func TestStatsPolicyWorkingAsIntended(t *testing.T) {
 
 		// compute the list of results we expect to see from the stats data
 		var expect []*httpsDialerTactic
-		idx := 0
 		for _, entry := range expectTacticsStats {
 			if entry.CountSuccess <= 0 || entry.Tactic == nil {
 				continue // we SHOULD NOT include entries that systematically failed
 			}
 			t := entry.Tactic.Clone()
-			t.InitialDelay = happyEyeballsDelay(idx)
+			t.InitialDelay = 0
 			expect = append(expect, t)
-			idx++
 		}
 
 		// perform the actual comparison
@@ -319,9 +313,9 @@ func (p *mocksPolicy) LookupTactics(ctx context.Context, domain string, port str
 	return p.MockLookupTactics(ctx, domain, port)
 }
 
-func TestStatsPolicyPostProcessTactics(t *testing.T) {
+func TestStatsPolicyFilterStatsTactics(t *testing.T) {
 	t.Run("we do nothing when good is false", func(t *testing.T) {
-		tactics := statsPolicyPostProcessTactics(nil, false)
+		tactics := statsPolicyFilterStatsTactics(nil, false)
 		if len(tactics) != 0 {
 			t.Fatal("expected zero-lenght return value")
 		}
@@ -390,7 +384,7 @@ func TestStatsPolicyPostProcessTactics(t *testing.T) {
 			},
 		}
 
-		got := statsPolicyPostProcessTactics(input, true)
+		got := statsPolicyFilterStatsTactics(input, true)
 
 		if len(got) != 1 {
 			t.Fatal("expected just one element")

internal/enginenetx/stream.go

@@ -0,0 +1,16 @@
+package enginenetx
+
+// streamTacticsFromSlice streams tactics from a given slice.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func streamTacticsFromSlice(input []*httpsDialerTactic) <-chan *httpsDialerTactic {
+	output := make(chan *httpsDialerTactic)
+	go func() {
+		defer close(output)
+		for _, tx := range input {
+			output <- tx
+		}
+	}()
+	return output
+}

internal/enginenetx/stream_test.go

@@ -0,0 +1,23 @@
+package enginenetx
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/ooni/probe-cli/v3/internal/testingx"
+)
+
+func TestStreamTacticsFromSlice(t *testing.T) {
+	input := []*httpsDialerTactic{}
+	ff := &testingx.FakeFiller{}
+	ff.Fill(&input)
+
+	var output []*httpsDialerTactic
+	for tx := range streamTacticsFromSlice(input) {
+		output = append(output, tx)
+	}
+
+	if diff := cmp.Diff(input, output); diff != "" {
+		t.Fatal(diff)
+	}
+}

internal/enginenetx/userpolicy.go

@@ -104,7 +104,7 @@ func (ldp *userPolicy) LookupTactics(
 		return ldp.Fallback.LookupTactics(ctx, domain, port)
 	}
 
-	// emit the resuults, which may possibly be empty
+	// emit the results, which may possibly be empty
 	out := make(chan *httpsDialerTactic)
 	go func() {
 		defer close(out) // let the caller know we're done