arnaucube
arnaucube/hermez-nodePublic Self-hosted repository
In http servers, first listen, then serve

- In test, doing the `net.Listen` outside of the goroutine guarantees that we
  are listening, so we avoid a possible datarace consisting of doing an http
  request before listening.
- In packages that run an http server: doing the listen first allows
    - Checking for errors when opening the address for listening before
      starting the goroutine, so that if there's an error on listen we can
      terminate grafecully
    - Logging that we are listening when we are really listening, and not
      before.
Eduard S eduard@iden3.io
Wed, 24 Feb 2021 12:00:38 +0100
commit

ca53dc9b52eb9d804552c84c53184faaffc051c9

parent

d16fba72a7ba5df2b1173403eab55e05c8c3ab2b

4 files changed, 40 insertions(+), 16 deletions(-)

jump to
M api/api_test.go → api/api_test.go 
@@ -8,6 +8,7 @@ "fmt"
 	"io"
 	"io/ioutil"
 	"math/big"
+	"net"
 	"net/http"
 	"os"
 	"strconv"

@@ -38,8 +39,8 @@ Len() int
 	New() Pendinger
 }
 
-const apiPort = ":4010"
-const apiURL = "http://localhost" + apiPort + "/"
+const apiAddr = ":4010"
+const apiURL = "http://localhost" + apiAddr + "/"
 
 var SetBlockchain = `
 	Type: Blockchain

@@ -241,9 +242,14 @@ if err != nil {
 		panic(err)
 	}
 	// Start server
-	server := &http.Server{Addr: apiPort, Handler: apiGin}
+	listener, err := net.Listen("tcp", apiAddr) //nolint:gosec
+	if err != nil {
+		panic(err)
+	}
+	server := &http.Server{Handler: apiGin}
 	go func() {
-		if err := server.ListenAndServe(); err != nil && tracerr.Unwrap(err) != http.ErrServerClosed {
+		if err := server.Serve(listener); err != nil &&
+			tracerr.Unwrap(err) != http.ErrServerClosed {
 			panic(err)
 		}
 	}()

@@ -609,9 +615,12 @@ startWait <- nil
 		<-finishWait
 	})
 	// Start server
-	serverTO := &http.Server{Addr: ":4444", Handler: apiGinTO}
+	serverTO := &http.Server{Handler: apiGinTO}
+	listener, err := net.Listen("tcp", ":4444") //nolint:gosec
+	require.NoError(t, err)
 	go func() {
-		if err := serverTO.ListenAndServe(); err != nil && tracerr.Unwrap(err) != http.ErrServerClosed {
+		if err := serverTO.Serve(listener); err != nil &&
+			tracerr.Unwrap(err) != http.ErrServerClosed {
 			require.NoError(t, err)
 		}
 	}()
M node/node.go → node/node.go 
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net"
 	"net/http"
 	"sync"
 	"time"

@@ -444,16 +445,20 @@ // Run starts the http server of the NodeAPI.  To stop it, pass a context with
 // cancelation.
 func (a *NodeAPI) Run(ctx context.Context) error {
 	server := &http.Server{
-		Addr:    a.addr,
 		Handler: a.engine,
 		// TODO: Figure out best parameters for production
 		ReadTimeout:    30 * time.Second, //nolint:gomnd
 		WriteTimeout:   30 * time.Second, //nolint:gomnd
 		MaxHeaderBytes: 1 << 20,          //nolint:gomnd
 	}
+	listener, err := net.Listen("tcp", a.addr)
+	if err != nil {
+		return tracerr.Wrap(err)
+	}
+	log.Infof("NodeAPI is ready at %v", a.addr)
 	go func() {
-		log.Infof("NodeAPI is ready at %v", a.addr)
-		if err := server.ListenAndServe(); err != nil && tracerr.Unwrap(err) != http.ErrServerClosed {
+		if err := server.Serve(listener); err != nil &&
+			tracerr.Unwrap(err) != http.ErrServerClosed {
 			log.Fatalf("Listen: %s\n", err)
 		}
 	}()
M test/debugapi/debugapi.go → test/debugapi/debugapi.go 
@@ -2,6 +2,7 @@ package debugapi
 
 import (
 	"context"
+	"net"
 	"net/http"
 	"time"
 

@@ -118,16 +119,20 @@
 	debugAPI.GET("sync/stats", a.handleSyncStats)
 
 	debugAPIServer := &http.Server{
-		Addr:    a.addr,
 		Handler: api,
-		// Use some hardcoded numberes that are suitable for testing
+		// Use some hardcoded numbers that are suitable for testing
 		ReadTimeout:    30 * time.Second, //nolint:gomnd
 		WriteTimeout:   30 * time.Second, //nolint:gomnd
 		MaxHeaderBytes: 1 << 20,          //nolint:gomnd
 	}
+	listener, err := net.Listen("tcp", a.addr)
+	if err != nil {
+		return tracerr.Wrap(err)
+	}
+	log.Infof("DebugAPI is ready at %v", a.addr)
 	go func() {
-		log.Infof("DebugAPI is ready at %v", a.addr)
-		if err := debugAPIServer.ListenAndServe(); err != nil && tracerr.Unwrap(err) != http.ErrServerClosed {
+		if err := debugAPIServer.Serve(listener); err != nil &&
+			tracerr.Unwrap(err) != http.ErrServerClosed {
 			log.Fatalf("Listen: %s\n", err)
 		}
 	}()
M test/proofserver/proofserver.go → test/proofserver/proofserver.go 
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"sync"
 	"time"

@@ -202,16 +203,20 @@ apiGroup.POST("/input", s.handleInput)
 	apiGroup.POST("/cancel", s.handleCancel)
 
 	debugAPIServer := &http.Server{
-		Addr:    s.addr,
 		Handler: api,
 		// Use some hardcoded numberes that are suitable for testing
 		ReadTimeout:    30 * time.Second, //nolint:gomnd
 		WriteTimeout:   30 * time.Second, //nolint:gomnd
 		MaxHeaderBytes: 1 << 20,          //nolint:gomnd
 	}
+	listener, err := net.Listen("tcp", s.addr)
+	if err != nil {
+		return tracerr.Wrap(err)
+	}
+	log.Infof("prover.MockServer is ready at %v", s.addr)
 	go func() {
-		log.Infof("prover.MockServer is ready at %v", s.addr)
-		if err := debugAPIServer.ListenAndServe(); err != nil && tracerr.Unwrap(err) != http.ErrServerClosed {
+		if err := debugAPIServer.Serve(listener); err != nil &&
+			tracerr.Unwrap(err) != http.ErrServerClosed {
 			log.Fatalf("Listen: %s\n", err)
 		}
 	}()