diff --git a/silence/silence.go b/silence/silence.go index 429cdb59..3c72eacb 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -370,7 +370,9 @@ func (s *Silences) setSilence(sil *pb.Silence) error { } s.st.Merge(st) - s.gossip.GossipBroadcast(st) + // setSilence() is called with s.mtx locked, which can produce + // a deadlock if we call GossipBroadcast from here. + go s.gossip.GossipBroadcast(st) return nil } diff --git a/silence/silence_test.go b/silence/silence_test.go index 2f3deea5..eed40604 100644 --- a/silence/silence_test.go +++ b/silence/silence_test.go @@ -197,18 +197,35 @@ func TestSilencesSetSilence(t *testing.T) { }, } - var called bool + done := make(chan bool) s.gossip = &mockGossip{ broadcast: func(d mesh.GossipData) { data, ok := d.(*gossipData) - require.True(t, ok, "gossip data of unknown type") - require.Equal(t, want, data, "unexpected gossip broadcast data") - called = true + // Double check that we can take a lock on s.mtx here. + s.mtx.Lock() + defer s.mtx.Unlock() + + require.True(t, ok, "gossip data of unknown type") + require.Equal(t, want.data, data.data, "unexpected gossip broadcast data") + close(done) }, } - require.NoError(t, s.setSilence(sil)) - require.True(t, called, "GossipBroadcast was not called") + + // setSilence() is always called with s.mtx locked() + go func() { + s.mtx.Lock() + require.NoError(t, s.setSilence(sil)) + s.mtx.Unlock() + }() + + // GossipBroadcast is called in a goroutine. + select { + case <-done: + case <-time.After(1 * time.Second): + t.Fatal("GossipBroadcast was not called") + } + require.Equal(t, want.data, s.st.data, "Unexpected silence state") }