Fix server shutdown race

services/horizon/internal/app.go

@@ -133,7 +133,7 @@ func (a *App) Serve() {
 	}()
 	go a.waitForDone()
 
-	err := a.webServer.Serve(uint16(a.config.Port), a.config.TLSCert, a.config.TLSKey, uint16(a.config.AdminPort))
+	err := a.webServer.Serve()
 	if err != nil && err != http.ErrServerClosed {
 		log.Fatal(err)
 	}
@@ -449,7 +449,7 @@ func (a *App) init() error {
 	// txsub.metrics
 	initTxSubMetrics(a)
 
-	webConfig := &httpx.RouterConfig{
+	routerConfig := httpx.RouterConfig{
 		DBSession:          a.historyQ.Session,
 		TxSubmitter:        a.submitter,
 		RateQuota:          a.config.RateQuota,
@@ -466,7 +466,17 @@ func (a *App) init() error {
 	}
 
 	var err error
-	a.webServer, err = httpx.NewServer(webConfig)
+	config := httpx.ServerConfig{
+		Port:      uint16(a.config.Port),
+		AdminPort: uint16(a.config.AdminPort),
+	}
+	if a.config.TLSCert != "" && a.config.TLSKey != "" {
+		config.TLSConfig = &httpx.TLSConfig{
+			CertPath: a.config.TLSCert,
+			KeyPath:  a.config.TLSKey,
+		}
+	}
+	a.webServer, err = httpx.NewServer(config, routerConfig)
 	if err != nil {
 		return err
 	}

services/horizon/internal/httpx/server.go

@@ -24,12 +24,22 @@ type ServerMetrics struct {
 	RequestDurationSummary *prometheus.SummaryVec
 }
 
-// Web contains the http server related fields for horizon: the Router,
+type TLSConfig struct {
+	CertPath, KeyPath string
+}
+type ServerConfig struct {
+	Port      uint16
+	TLSConfig *TLSConfig
+	AdminPort uint16
+}
+
+// Server contains the http server related fields for horizon: the Router,
 // rate limiter, etc.
 type Server struct {
 	Router         *Router
 	Metrics        *ServerMetrics
 	server         *http.Server
+	config         ServerConfig
 	internalServer *http.Server
 }
 
@@ -46,7 +56,7 @@ func init() {
 	problem.RegisterError(db.ErrCancelled, hProblem.ServiceUnavailable)
 }
 
-func NewServer(config *RouterConfig) (*Server, error) {
+func NewServer(serverConfig ServerConfig, routerConfig RouterConfig) (*Server, error) {
 	sm := &ServerMetrics{
 		RequestDurationSummary: prometheus.NewSummaryVec(
 			prometheus.SummaryOpts{
@@ -56,47 +66,44 @@ func NewServer(config *RouterConfig) (*Server, error) {
 			[]string{"status", "route", "streaming", "method"},
 		),
 	}
-
-	router, err := NewRouter(config, sm)
+	router, err := NewRouter(&routerConfig, sm)
 	if err != nil {
 		return nil, err
 	}
+	addr := fmt.Sprintf(":%d", serverConfig.Port)
 	result := &Server{
 		Router:  router,
 		Metrics: sm,
+		server: &http.Server{
+			Addr:        addr,
+			Handler:     router,
+			ReadTimeout: 5 * time.Second,
+		},
+	}
+
+	if serverConfig.AdminPort != 0 {
+		adminAddr := fmt.Sprintf(":%d", serverConfig.AdminPort)
+		result.internalServer = &http.Server{
+			Addr:        adminAddr,
+			Handler:     result.Router.Internal,
+			ReadTimeout: 5 * time.Second,
+		}
 	}
 	return result, nil
 }
-func (s *Server) Serve(port uint16, certFile, keyFile string, adminPort uint16) error {
-	if s.server != nil {
-		return errors.New("server already started")
-	}
-
-	if adminPort != 0 {
+func (s *Server) Serve() error {
+	if s.internalServer != nil {
 		go func() {
-			adminAddr := fmt.Sprintf(":%d", adminPort)
-			log.Infof("Starting internal server on %s", adminAddr)
-			s.internalServer = &http.Server{
-				Addr:        adminAddr,
-				Handler:     s.Router.Internal,
-				ReadTimeout: 5 * time.Second,
-			}
+			log.Infof("Starting internal server on %s", s.internalServer.Addr)
 			if err := s.internalServer.ListenAndServe(); err != nil && err != http.ErrServerClosed {
 				log.Warn(errors.Wrap(err, "error in internalServer.ListenAndServe()"))
 			}
 		}()
 	}
 
-	addr := fmt.Sprintf(":%d", port)
-	s.server = &http.Server{
-		Addr:        addr,
-		Handler:     s.Router,
-		ReadTimeout: 5 * time.Second,
-	}
-
 	var err error
-	if certFile != "" {
-		err = s.server.ListenAndServeTLS(certFile, keyFile)
+	if s.config.TLSConfig != nil {
+		err = s.server.ListenAndServeTLS(s.config.TLSConfig.CertPath, s.config.TLSConfig.KeyPath)
 	} else {
 		err = s.server.ListenAndServe()
 	}