Ensure sig handler routine returns on shutdown, turn it off in most t…

…ests

I noticed that when running the test suite, there would be a file
server/log1.txt left. This file is created by one of the config
reload test. Running this test individually was doing the proper
cleanup. I noticed that the Signal test that was checking
that files could be rotated was causing this side effect.
It turns out that none of the config reload tests were disabling
the signal handler (NoSigs=true), and since the go routine would
be left running, running the TestSignalToReOpenLogFile() test
would interact with an already finished test.

I put a thread dump in handleSignals() to track all tests that
were causing this function to start the go routine because NoSigs
was not set to true. I fixed all those tests. At this time, there
are only 2 tests that need to start the signal handler.

I have also fixed the code so that the signal handler routine select
on a server quitCh that is closed on shutdown so that this go routine
exit and is waiting on using the grWG wait group.

server/client_test.go

@@ -698,6 +698,7 @@ func TestTLSCloseClientConnection(t *testing.T) {
 	}
 	opts.TLSTimeout = 100
 	opts.NoLog = true
+	opts.NoSigs = true
 	s := RunServer(opts)
 	defer s.Shutdown()
 

server/reload.go

@@ -570,8 +570,8 @@ func (s *Server) diffOptions(newOpts *Options) ([]option, error) {
 				}
 			}
 			diffOpts = append(diffOpts, &clientAdvertiseOption{newValue: cliAdv})
-		case "nolog":
-			// Ignore NoLog option since it's not parsed and only used in
+		case "nolog", "nosigs":
+			// Ignore NoLog ad NoSigs options since they are not parsed and only used in
 			// testing.
 			continue
 		case "port":

server/reload_test.go

@@ -32,7 +32,7 @@ import (
 // Ensure Reload returns an error when attempting to reload a server that did
 // not start with a config file.
 func TestConfigReloadNoConfigFile(t *testing.T) {
-	server := New(&Options{})
+	server := New(&Options{NoSigs: true})
 	loaded := server.ConfigTime()
 	if server.Reload() == nil {
 		t.Fatal("Expected Reload to return an error")
@@ -69,6 +69,7 @@ func TestConfigReloadUnsupported(t *testing.T) {
 			Host: "localhost",
 			Port: -1,
 		},
+		NoSigs: true,
 	}
 	processOptions(golden)
 
@@ -158,6 +159,7 @@ func TestConfigReloadInvalidConfig(t *testing.T) {
 			Host: "localhost",
 			Port: -1,
 		},
+		NoSigs: true,
 	}
 	processOptions(golden)
 
@@ -224,6 +226,7 @@ func TestConfigReload(t *testing.T) {
 			Host: "localhost",
 			Port: server.ClusterAddr().Port,
 		},
+		NoSigs: true,
 	}
 	processOptions(golden)
 
@@ -1306,6 +1309,7 @@ func TestConfigReloadClusterRoutes(t *testing.T) {
 		t.Fatalf("Error processing config file: %v", err)
 	}
 	srvcOpts.NoLog = true
+	srvcOpts.NoSigs = true
 
 	srvc := RunServer(srvcOpts)
 	defer srvc.Shutdown()
@@ -1410,6 +1414,7 @@ func TestConfigReloadClusterAdvertise(t *testing.T) {
 		t.Fatalf("Error processing config file: %v", err)
 	}
 	opts.NoLog = true
+	opts.NoSigs = true
 	s := RunServer(opts)
 	defer s.Shutdown()
 
@@ -1491,6 +1496,7 @@ func TestConfigReloadClusterNoAdvertise(t *testing.T) {
 		t.Fatalf("Error processing config file: %v", err)
 	}
 	opts.NoLog = true
+	opts.NoSigs = true
 	s := RunServer(opts)
 	defer s.Shutdown()
 
@@ -1545,6 +1551,7 @@ func TestConfigReloadClientAdvertise(t *testing.T) {
 		stackFatalf(t, "Error processing config file: %v", err)
 	}
 	opts.NoLog = true
+	opts.NoSigs = true
 	s := RunServer(opts)
 	defer s.Shutdown()
 
@@ -1767,11 +1774,13 @@ func TestConfigReloadRotateFiles(t *testing.T) {
 func runServerWithSymlinkConfig(t *testing.T, symlinkName, configName string) (*Server, *Options, string) {
 	opts, config := newOptionsWithSymlinkConfig(t, symlinkName, configName)
 	opts.NoLog = true
+	opts.NoSigs = true
 	return RunServer(opts), opts, config
 }
 
 func newServerWithSymlinkConfig(t *testing.T, symlinkName, configName string) (*Server, *Options, string) {
 	opts, config := newOptionsWithSymlinkConfig(t, symlinkName, configName)
+	opts.NoSigs = true
 	return New(opts), opts, config
 }
 
@@ -1786,6 +1795,7 @@ func newOptionsWithSymlinkConfig(t *testing.T, symlinkName, configName string) (
 	if err != nil {
 		t.Fatalf("Error processing config file: %v", err)
 	}
+	opts.NoSigs = true
 	return opts, config
 }
 

server/route.go

@@ -812,7 +812,7 @@ func (s *Server) connectToRoute(rURL *url.URL, tryForEver bool) {
 				}
 			}
 			select {
-			case <-s.rcQuit:
+			case <-s.quitCh:
 				return
 			case <-time.After(DEFAULT_ROUTE_CONNECT):
 				continue

server/routes_test.go

@@ -542,6 +542,7 @@ func TestTLSChainedSolicitWorks(t *testing.T) {
 func TestRouteTLSHandshakeError(t *testing.T) {
 	optsSeed, _ := ProcessConfigFile("./configs/seed_tls.conf")
 	optsSeed.NoLog = true
+	optsSeed.NoSigs = true
 	srvSeed := RunServer(optsSeed)
 	defer srvSeed.Shutdown()
 

server/server.go

@@ -83,7 +83,7 @@ type Server struct {
 	routeListener net.Listener
 	routeInfo     Info
 	routeInfoJSON []byte
-	rcQuit        chan bool
+	quitCh        chan struct{}
 	grMu          sync.Mutex
 	grTmpClients  map[uint64]*client
 	grRunning     bool
@@ -173,9 +173,9 @@ func New(opts *Options) *Server {
 	s.routes = make(map[uint64]*client)
 	s.remotes = make(map[string]*client)
 
-	// Used to kick out all of the route
-	// connect Go routines.
-	s.rcQuit = make(chan bool)
+	// Used to kick out all go routines possibly waiting on server
+	// to shutdown.
+	s.quitCh = make(chan struct{})
 
 	// Used to setup Authorization.
 	s.configureAuthorization()
@@ -381,8 +381,8 @@ func (s *Server) Shutdown() {
 		s.profiler.Close()
 	}
 
-	// Release the solicited routes connect go routines.
-	close(s.rcQuit)
+	// Release go routines that wait on that channel
+	close(s.quitCh)
 
 	s.mu.Unlock()
 

server/signal.go

@@ -37,21 +37,28 @@ func (s *Server) handleSignals() {
 
 	signal.Notify(c, syscall.SIGINT, syscall.SIGUSR1, syscall.SIGHUP)
 
+	s.grWG.Add(1)
 	go func() {
-		for sig := range c {
-			s.Debugf("Trapped %q signal", sig)
-			switch sig {
-			case syscall.SIGINT:
-				s.Noticef("Server Exiting..")
-				os.Exit(0)
-			case syscall.SIGUSR1:
-				// File log re-open for rotating file logs.
-				s.ReOpenLogFile()
-			case syscall.SIGHUP:
-				// Config reload.
-				if err := s.Reload(); err != nil {
-					s.Errorf("Failed to reload server configuration: %s", err)
+		defer s.grWG.Done()
+		for {
+			select {
+			case sig := <-c:
+				s.Debugf("Trapped %q signal", sig)
+				switch sig {
+				case syscall.SIGINT:
+					s.Noticef("Server Exiting..")
+					os.Exit(0)
+				case syscall.SIGUSR1:
+					// File log re-open for rotating file logs.
+					s.ReOpenLogFile()
+				case syscall.SIGHUP:
+					// Config reload.
+					if err := s.Reload(); err != nil {
+						s.Errorf("Failed to reload server configuration: %s", err)
+					}
 				}
+			case <-s.quitCh:
+				return
 			}
 		}
 	}()

test/port_test.go

@@ -22,7 +22,7 @@ import (
 )
 
 func TestResolveRandomPort(t *testing.T) {
-	opts := &server.Options{Host: "127.0.0.1", Port: server.RANDOM_PORT}
+	opts := &server.Options{Host: "127.0.0.1", Port: server.RANDOM_PORT, NoSigs: true}
 	s := RunServer(opts)
 	defer s.Shutdown()