Fix gathering candidates race condition

Canceling previous gathering does not happen synchronously. If it takes
a little bit longer, it can close newly created `gatherCandidateDone`
channel. If we pass channel to the `gatherCandidates` function, it will
always close its done channel and only the latest done channel will be
available in `gatherCandidateDone`.
This commit is contained in:
Miroslav Šedivý
2023-03-28 12:26:58 -04:00
committed by Sean DuBois
parent be72bc709b
commit cf6f758882
2 changed files with 33 additions and 4 deletions
+5 -4
View File
@@ -51,17 +51,18 @@ func (a *Agent) GatherCandidates() error {
a.gatherCandidateCancel() // Cancel previous gathering routine
ctx, cancel := context.WithCancel(ctx)
a.gatherCandidateCancel = cancel
a.gatherCandidateDone = make(chan struct{})
done := make(chan struct{})
a.gatherCandidateDone = done
go a.gatherCandidates(ctx)
go a.gatherCandidates(ctx, done)
}); runErr != nil {
return runErr
}
return gatherErr
}
func (a *Agent) gatherCandidates(ctx context.Context) {
defer close(a.gatherCandidateDone)
func (a *Agent) gatherCandidates(ctx context.Context, done chan struct{}) {
defer close(done)
if err := a.setGatheringState(GatheringStateGathering); err != nil { //nolint:contextcheck
a.log.Warnf("failed to set gatheringState to GatheringStateGathering: %v", err)
return
+28
View File
@@ -87,6 +87,34 @@ func TestListenUDP(t *testing.T) {
assert.NoError(t, a.Close())
}
func TestGatherConcurrency(t *testing.T) {
report := test.CheckRoutines(t)
defer report()
lim := test.TimeOut(time.Second * 30)
defer lim.Stop()
a, err := NewAgent(&AgentConfig{
NetworkTypes: []NetworkType{NetworkTypeUDP4, NetworkTypeUDP6},
IncludeLoopback: true,
})
assert.NoError(t, err)
candidateGathered, candidateGatheredFunc := context.WithCancel(context.Background())
assert.NoError(t, a.OnCandidate(func(c Candidate) {
candidateGatheredFunc()
}))
// tesing for panic
for i := 0; i < 10; i++ {
_ = a.GatherCandidates()
}
<-candidateGathered.Done()
assert.NoError(t, a.Close())
}
func TestLoopbackCandidate(t *testing.T) {
report := test.CheckRoutines(t)
defer report()