Merge pull request #935 from bboreham/weavedns-races

All unit and smoke tests passed.

common/logging.go

@@ -19,6 +19,7 @@ var (
 	Info    *log.Logger
 	Warning *log.Logger
 	Error   *log.Logger
+	debugF  bool
 )
 
 func InitLogging(debugHandle io.Writer,
@@ -33,6 +34,10 @@ func InitLogging(debugHandle io.Writer,
 }
 
 func InitDefaultLogging(debug bool) {
+	if debug == debugF {
+		return
+	}
+	debugF = debug
 	debugOut := ioutil.Discard
 	if debug {
 		debugOut = os.Stderr

nameserver/cache.go

@@ -155,7 +155,7 @@ func (e *cacheEntry) setReply(reply *dns.Msg, ttl int, flags uint8, now time.Tim
 	e.putTime = now
 
 	if reply != nil {
-		e.reply = *reply
+		e.reply = *reply.Copy()
 		e.ReplyLen = reply.Len()
 	}
 

nameserver/mdns_client.go

@@ -97,7 +97,6 @@ type MDNSClient struct {
 	addr       *net.UDPAddr
 	inflight   map[string]*inflightQuery
 	actionChan chan<- MDNSAction
-	running    bool
 }
 
 type mDNSQueryInfo struct {
@@ -109,15 +108,10 @@ type mDNSQueryInfo struct {
 func NewMDNSClient() (*MDNSClient, error) {
 	return &MDNSClient{
 		addr:     ipv4Addr,
-		running:  false,
 		inflight: make(map[string]*inflightQuery)}, nil
 }
 
 func (c *MDNSClient) Start(ifi *net.Interface) (err error) {
-	if c.running {
-		return nil
-	}
-
 	if c.conn, err = net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero, Port: 0}); err != nil {
 		return err
 	}
@@ -134,19 +128,18 @@ func (c *MDNSClient) Start(ifi *net.Interface) (err error) {
 		}
 	}
 
-	c.listener = &dns.Server{Unsafe: true, PacketConn: multicast, Handler: dns.HandlerFunc(handleMDNS)}
-	go c.listener.ActivateAndServe()
-
 	actionChan := make(chan MDNSAction, MailboxSize)
 	c.actionChan = actionChan
 	go c.actorLoop(actionChan)
+
+	c.listener = &dns.Server{Unsafe: true, PacketConn: multicast, Handler: dns.HandlerFunc(handleMDNS)}
+	go c.listener.ActivateAndServe()
+
 	return nil
 }
 
 func (c *MDNSClient) Stop() error {
-	if c.running {
-		c.actionChan <- nil
-	}
+	c.actionChan <- nil
 	return nil
 }
 
@@ -272,13 +265,13 @@ func (c *MDNSClient) checkInFlightQueries() time.Duration {
 func (c *MDNSClient) actorLoop(actionChan <-chan MDNSAction) {
 	timer := time.NewTimer(MaxDuration)
 	run := func() { timer.Reset(c.checkInFlightQueries()) }
-	c.running = true
-	for c.running {
+loop:
+	for {
 		select {
 		case action := <-actionChan:
 			if action == nil {
 				c.listener.Shutdown()
-				c.running = false
+				break loop
 			} else {
 				action()
 				run()

nameserver/mdns_server.go

@@ -14,7 +14,6 @@ type MDNSServer struct {
 	zone       Zone
 	allowLocal bool
 	ttl        int
-	running    bool
 }
 
 // Create a new mDNS server
@@ -43,11 +42,6 @@ func (s *MDNSServer) addrIsLocal(testaddr net.Addr) bool {
 
 // Start the mDNS server
 func (s *MDNSServer) Start(ifi *net.Interface) (err error) {
-	// skip double initialization
-	if s.running {
-		return nil
-	}
-
 	// This is a bit of a kludge - per the RFC we should send responses from 5353, but that doesn't seem to work
 	s.sendconn, err = net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero, Port: 0})
 	if err != nil {
@@ -94,13 +88,11 @@ func (s *MDNSServer) Start(ifi *net.Interface) (err error) {
 		Handler:    mux,
 	}
 	go s.srv.ActivateAndServe()
-	s.running = true
 	return err
 }
 
 // Stop the mDNS server
 func (s *MDNSServer) Stop() error {
-	s.running = false
 	return s.srv.Shutdown()
 }
 

nameserver/mdns_server_test.go

@@ -41,16 +41,7 @@ func TestServerSimpleQuery(t *testing.T) {
 		if len(r.Answer) > 0 {
 			t.Logf("Received %d answer(s)", len(r.Answer))
 			for _, answer := range r.Answer {
-				switch rr := answer.(type) {
-				case *dns.A:
-					t.Logf("... A:\n%+v", rr)
-					receivedAddrs = append(receivedAddrs, rr.A)
-					receivedCount++
-				case *dns.PTR:
-					t.Logf("... PTR:\n%+v", rr)
-					receivedName = rr.Ptr
-					receivedCount++
-				}
+				recvChan <- answer
 			}
 			recvChan <- "ok"
 		}
@@ -60,7 +51,6 @@ func TestServerSimpleQuery(t *testing.T) {
 		receivedAddrs = make([]net.IP, 0)
 		receivedName = ""
 		receivedCount = 0
-		recvChan = make(chan interface{})
 
 		m := new(dns.Msg)
 		m.SetQuestion(name, querytype)
@@ -74,12 +64,25 @@ func TestServerSimpleQuery(t *testing.T) {
 		wt.AssertNoErr(t, err)
 
 		Debug.Printf("Waiting for response")
-		select {
-		case <-recvChan:
-			return
-		case <-time.After(100 * time.Millisecond):
-			Debug.Printf("Timeout while waiting for response")
-			return
+		for {
+			select {
+			case x := <-recvChan:
+				switch rr := x.(type) {
+				case *dns.A:
+					t.Logf("... A:\n%+v", rr)
+					receivedAddrs = append(receivedAddrs, rr.A)
+					receivedCount++
+				case *dns.PTR:
+					t.Logf("... PTR:\n%+v", rr)
+					receivedName = rr.Ptr
+					receivedCount++
+				case string:
+					return
+				}
+			case <-time.After(100 * time.Millisecond):
+				Debug.Printf("Timeout while waiting for response")
+				return
+			}
 		}
 	}
 

nameserver/server.go

@@ -218,7 +218,10 @@ func (s *DNSServer) Start() error {
 	s.tcpSrv = &dns.Server{Listener: l, Handler: s.createMux(protTCP), ReadTimeout: s.readTimeout}
 
 	s.listenersWg.Add(2)
+	return nil
+}
 
+func (s *DNSServer) ActivateAndServe() {
 	go func() {
 		defer s.listenersWg.Done()
 
@@ -241,7 +244,6 @@ func (s *DNSServer) Start() error {
 	s.listenersWg.Wait()
 
 	Info.Printf("[dns] Server exiting...")
-	return nil
 }
 
 // Return status string

nameserver/server_cache_test.go

@@ -69,7 +69,9 @@ func TestServerDbCacheInvalidation(t *testing.T) {
 		MaxAnswers:        4,
 	})
 	wt.AssertNoErr(t, err)
-	go srv.Start()
+	err = srv.Start()
+	wt.AssertNoErr(t, err)
+	go srv.ActivateAndServe()
 	defer srv.Stop()
 	time.Sleep(100 * time.Millisecond) // Allow server goroutine to start
 
@@ -202,7 +204,9 @@ func TestServerCacheExpiration(t *testing.T) {
 		CacheNegLocalTTL:  negativeLocalTTL,
 	})
 	wt.AssertNoErr(t, err)
-	go srv.Start()
+	err = srv.Start()
+	wt.AssertNoErr(t, err)
+	go srv.ActivateAndServe()
 	defer srv.Stop()
 	time.Sleep(100 * time.Millisecond) // Allow server goroutine to start
 
@@ -287,7 +291,9 @@ func TestServerCacheRefresh(t *testing.T) {
 		MaxAnswers:        4,
 	})
 	wt.AssertNoErr(t, err)
-	go srv.Start()
+	err = srv.Start()
+	wt.AssertNoErr(t, err)
+	go srv.ActivateAndServe()
 	defer srv.Stop()
 	time.Sleep(100 * time.Millisecond) // Allow sever goroutine to start
 

nameserver/server_test.go

@@ -89,7 +89,9 @@ func TestUDPDNSServer(t *testing.T) {
 		ListenReadTimeout: testSocketTimeout,
 	})
 	wt.AssertNoErr(t, err)
-	go srv.Start()
+	err = srv.Start()
+	wt.AssertNoErr(t, err)
+	go srv.ActivateAndServe()
 	defer srv.Stop()
 	time.Sleep(100 * time.Millisecond) // Allow sever goroutine to start
 
@@ -195,7 +197,9 @@ func TestTCPDNSServer(t *testing.T) {
 		ListenReadTimeout: testSocketTimeout,
 	})
 	wt.AssertNoErr(t, err)
-	go srv.Start()
+	err = srv.Start()
+	wt.AssertNoErr(t, err)
+	go srv.ActivateAndServe()
 	defer srv.Stop()
 	time.Sleep(100 * time.Millisecond) // Allow sever goroutine to start
 

prog/weavedns/main.go

@@ -182,4 +182,5 @@ func main() {
 	if err != nil {
 		Error.Fatal("[main] Failed to start the WeaveDNS server: ", err)
 	}
+	srv.ActivateAndServe()
 }