Assert that agent isn't started twice

Check was lost, also add tests so we don't regress again

agent.go

@@ -4,11 +4,11 @@ package ice
 
 import (
 	"bytes"
-	"fmt"
 	"math/rand"
 	"net"
 	"sort"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/pion/logging"
@@ -68,7 +68,7 @@ type Agent struct {
 	connectionState ConnectionState
 	gatheringState  GatheringState
 
-	haveStarted   bool
+	haveStarted   atomic.Value
 	isControlling bool
 
 	portmin uint16
@@ -186,6 +186,7 @@ func NewAgent(config *AgentConfig) (*Agent, error) {
 
 		forceCandidateContact: make(chan bool, 1),
 	}
+	a.haveStarted.Store(false)
 
 	// Make sure the buffer doesn't grow indefinitely.
 	// NOTE: We actually won't get anywhere close to this limit.
@@ -244,13 +245,14 @@ func (a *Agent) onSelectedCandidatePairChange(p *candidatePair) {
 
 func (a *Agent) startConnectivityChecks(isControlling bool, remoteUfrag, remotePwd string) error {
 	switch {
-	case a.haveStarted:
-		return fmt.Errorf("attempted to start agent twice")
+	case a.haveStarted.Load():
+		return ErrMultipleStart
 	case remoteUfrag == "":
-		return fmt.Errorf("remoteUfrag is empty")
+		return ErrRemoteUfragEmpty
 	case remotePwd == "":
-		return fmt.Errorf("remotePwd is empty")
+		return ErrRemotePwdEmpty
 	}
+	a.haveStarted.Store(true)
 	a.log.Debugf("Started agent: isControlling? %t, remoteUfrag: %q, remotePwd: %q", isControlling, remoteUfrag, remotePwd)
 
 	return a.run(func(agent *Agent) {

agent_test.go

@@ -1,6 +1,7 @@
 package ice
 
 import (
+	"context"
 	"net"
 	"testing"
 	"time"
@@ -465,3 +466,30 @@ func TestInboundValidity(t *testing.T) {
 		}
 	})
 }
+
+func TestInvalidAgentStarts(t *testing.T) {
+	a, err := NewAgent(&AgentConfig{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ctx := context.Background()
+	ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond)
+	defer cancel()
+
+	if _, err = a.Dial(ctx, "", "bar"); err != nil && err != ErrRemoteUfragEmpty {
+		t.Fatal(err)
+	}
+
+	if _, err = a.Dial(ctx, "foo", ""); err != nil && err != ErrRemotePwdEmpty {
+		t.Fatal(err)
+	}
+
+	if _, err = a.Dial(ctx, "foo", "bar"); err != nil && err != ErrCanceledByCaller {
+		t.Fatal(err)
+	}
+
+	if _, err = a.Dial(context.TODO(), "foo", "bar"); err != nil && err != ErrMultipleStart {
+		t.Fatal(err)
+	}
+}

errors.go

@@ -29,4 +29,16 @@ var (
 
 	// ErrNoCandidatePairs indicates agent does not have a valid candidate pair
 	ErrNoCandidatePairs = errors.New("no candidate pairs available")
+
+	// ErrCanceledByCaller indicates agent connection was canceled by the caller
+	ErrCanceledByCaller = errors.New("connecting canceled by caller")
+
+	// ErrMultipleStart indicates agent was started twice
+	ErrMultipleStart = errors.New("attempted to start agent twice")
+
+	// ErrRemoteUfragEmpty indicates agent was started with an empty remote ufrag
+	ErrRemoteUfragEmpty = errors.New("remote ufrag is empty")
+
+	// ErrRemotePwdEmpty indicates agent was started with an empty remote pwd
+	ErrRemotePwdEmpty = errors.New("remote pwd is empty")
 )

transport.go

@@ -44,7 +44,7 @@ func (a *Agent) connect(ctx context.Context, isControlling bool, remoteUfrag, re
 	select {
 	case <-ctx.Done():
 		// TODO: Stop connectivity checks?
-		return nil, errors.New("connecting canceled by caller")
+		return nil, ErrCanceledByCaller
 	case <-a.onConnected:
 	}