Server: drop inflight waitgroup (#561)

* Server: drop inflight waitgroup

This drops the waitgroup in Server, the suspicion is this can make the server
fail to stop; doing this make graceful shutdown not work.

Add test that tries to find a race between starting on stopping race;
there was a data race on srv.Inflight.

The coredns' TestReadme doesn't race anymore with this as it did with
the more evasive PR #546.

client_test.go

@@ -525,7 +525,7 @@ func TestTimeout(t *testing.T) {
 	length := time.Since(start)
 
 	if length > allowable {
-		t.Errorf("exchange took longer (%v) than specified Timeout (%v)", length, timeout)
+		t.Errorf("exchange took longer %v than specified Timeout %v", length, allowable)
 	}
 }
 

server.go

@@ -297,10 +297,7 @@ type Server struct {
 	// DecorateWriter is optional, allows customization of the process that writes raw DNS messages.
 	DecorateWriter DecorateWriter
 
-	// Graceful shutdown handling
+	// Shutdown handling
-
-	inFlight sync.WaitGroup
-
 	lock    sync.RWMutex
 	started bool
 }
@@ -412,10 +409,8 @@ func (srv *Server) ActivateAndServe() error {
 	return &Error{err: "bad listeners"}
 }
 
-// Shutdown gracefully shuts down a server. After a call to Shutdown, ListenAndServe and
+// Shutdown shuts down a server. After a call to Shutdown, ListenAndServe and
-// ActivateAndServe will return. All in progress queries are completed before the server
+// ActivateAndServe will return.
-// is taken down. If the Shutdown is taking longer than the reading timeout an error
-// is returned.
 func (srv *Server) Shutdown() error {
 	srv.lock.Lock()
 	if !srv.started {
@@ -431,19 +426,7 @@ func (srv *Server) Shutdown() error {
 	if srv.Listener != nil {
 		srv.Listener.Close()
 	}
-
+	return nil
-	fin := make(chan bool)
-	go func() {
-		srv.inFlight.Wait()
-		fin <- true
-	}()
-
-	select {
-	case <-time.After(srv.getReadTimeout()):
-		return &Error{err: "server shutdown is pending"}
-	case <-fin:
-		return nil
-	}
 }
 
 // getReadTimeout is a helper func to use system timeout if server did not intend to change it.
@@ -493,7 +476,6 @@ func (srv *Server) serveTCP(l net.Listener) error {
 		if err != nil {
 			continue
 		}
-		srv.inFlight.Add(1)
 		go srv.serve(rw.RemoteAddr(), handler, m, nil, nil, rw)
 	}
 }
@@ -529,15 +511,12 @@ func (srv *Server) serveUDP(l *net.UDPConn) error {
 		if err != nil {
 			continue
 		}
-		srv.inFlight.Add(1)
 		go srv.serve(s.RemoteAddr(), handler, m, l, s, nil)
 	}
 }
 
 // Serve a new connection.
 func (srv *Server) serve(a net.Addr, h Handler, m []byte, u *net.UDPConn, s *SessionUDP, t net.Conn) {
-	defer srv.inFlight.Done()
-
 	w := &response{tsigSecret: srv.TsigSecret, udp: u, tcp: t, remoteAddr: a, udpSession: s}
 	if srv.DecorateWriter != nil {
 		w.writer = srv.DecorateWriter(w)

server_test.go

@@ -648,6 +648,22 @@ func TestShutdownUDP(t *testing.T) {
 	}
 }
 
+func TestServerStartStopRace(t *testing.T) {
+	for i := 0; i < 10; i++ {
+		var err error
+		s := &Server{}
+		s, _, _, err = RunLocalUDPServerWithFinChan(":0")
+		if err != nil {
+			t.Fatalf("Could not start server: %s", err)
+		}
+		go func() {
+			if err := s.Shutdown(); err != nil {
+				t.Fatalf("Could not stop server: %s", err)
+			}
+		}()
+	}
+}
+
 type ExampleFrameLengthWriter struct {
 	Writer
 }