Skip to content

Commit

Permalink
fix: change net.GetFreePort to listening on port 0 (#530)
Browse files Browse the repository at this point in the history
  • Loading branch information
thehowl authored Feb 27, 2023
1 parent 3aa1d81 commit 3291945
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 79 deletions.
22 changes: 22 additions & 0 deletions pkgs/bft/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,13 @@ func (n *Node) OnStart() error {
if err := n.transport.Listen(*addr); err != nil {
return err
}
if addr.Port == 0 {
// if the port we have from config.P2p.ListenAdress is 0,
// it means the port was selected when doing net.Listen (using autoselect on the kernel).
// fix the config variable using the correct address
na := n.transport.NetAddress()
n.config.P2P.ListenAddress = na.DialString()
}

n.isListening = true

Expand Down Expand Up @@ -687,6 +694,7 @@ func (n *Node) startRPC() ([]net.Listener, error) {
}

// we may expose the rpc over both a unix and tcp socket
var rebuildAddresses bool
listeners := make([]net.Listener, len(listenAddrs))
for i, listenAddr := range listenAddrs {
mux := http.NewServeMux()
Expand All @@ -702,6 +710,9 @@ func (n *Node) startRPC() ([]net.Listener, error) {
wm.SetLogger(wmLogger)
mux.HandleFunc("/websocket", wm.WebsocketHandler)
rpcserver.RegisterRPCFuncs(mux, rpccore.Routes, rpcLogger)
if strings.HasPrefix(listenAddr, "tcp://") && strings.HasSuffix(listenAddr, ":0") {
rebuildAddresses = true
}
listener, err := rpcserver.Listen(
listenAddr,
config,
Expand Down Expand Up @@ -739,10 +750,21 @@ func (n *Node) startRPC() ([]net.Listener, error) {

listeners[i] = listener
}
if rebuildAddresses {
n.config.RPC.ListenAddress = joinListenerAddresses(listeners)
}

return listeners, nil
}

func joinListenerAddresses(ll []net.Listener) string {
sl := make([]string, len(ll))
for i, l := range ll {
sl[i] = l.Addr().Network() + "://" + l.Addr().String()
}
return strings.Join(sl, ",")
}

// Switch returns the Node's Switch.
func (n *Node) Switch() *p2p.Switch {
return n.sw
Expand Down
2 changes: 1 addition & 1 deletion pkgs/bft/privval/signer_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func getSignerTestCases(t *testing.T) []signerTestCase {
mockPV := types.NewMockPV()

// get a pair of signer listener, signer dialer endpoints
sl, sd := getMockEndpoints(t, dtc.addr, dtc.dialer)
sl, sd := getMockEndpoints(t, dtc.listener, dtc.dialer)
sc, err := NewSignerClient(sl)
require.NoError(t, err)
ss := NewSignerServer(sd, chainID, mockPV)
Expand Down
23 changes: 7 additions & 16 deletions pkgs/bft/privval/signer_listener_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/gnolang/gno/pkgs/bft/types"
"github.com/gnolang/gno/pkgs/crypto/ed25519"
"github.com/gnolang/gno/pkgs/log"
osm "github.com/gnolang/gno/pkgs/os"
"github.com/gnolang/gno/pkgs/random"
)

Expand All @@ -23,8 +22,8 @@ var (
)

type dialerTestCase struct {
addr string
dialer SocketDialer
listener net.Listener
dialer SocketDialer
}

// TestSignerRemoteRetryTCPOnly will test connection retry attempts over TCP. We
Expand Down Expand Up @@ -91,7 +90,7 @@ func TestRetryConnToRemoteSigner(t *testing.T) {
mockPV = types.NewMockPV()
endpointIsOpenCh = make(chan struct{})
thisConnTimeout = testTimeoutReadWrite
listenerEndpoint = newSignerListenerEndpoint(logger, tc.addr, thisConnTimeout)
listenerEndpoint = newSignerListenerEndpoint(logger, tc.listener, thisConnTimeout)
)

dialerEndpoint := NewSignerDialerEndpoint(
Expand Down Expand Up @@ -133,18 +132,10 @@ func TestRetryConnToRemoteSigner(t *testing.T) {

// /////////////////////////////////

func newSignerListenerEndpoint(logger log.Logger, addr string, timeoutReadWrite time.Duration) *SignerListenerEndpoint {
proto, address := osm.ProtocolAndAddress(addr)

ln, err := net.Listen(proto, address)
logger.Info("SignerListener: Listening", "proto", proto, "address", address)
if err != nil {
panic(err)
}

func newSignerListenerEndpoint(logger log.Logger, ln net.Listener, timeoutReadWrite time.Duration) *SignerListenerEndpoint {
var listener net.Listener

if proto == "unix" {
if ln.Addr().Network() == "unix" {
unixLn := NewUnixListener(ln)
UnixListenerTimeoutAccept(testTimeoutAccept)(unixLn)
UnixListenerTimeoutReadWrite(timeoutReadWrite)(unixLn)
Expand All @@ -171,7 +162,7 @@ func startListenerEndpointAsync(t *testing.T, sle *SignerListenerEndpoint, endpo

func getMockEndpoints(
t *testing.T,
addr string,
l net.Listener,
socketDialer SocketDialer,
) (*SignerListenerEndpoint, *SignerDialerEndpoint) {
t.Helper()
Expand All @@ -185,7 +176,7 @@ func getMockEndpoints(
socketDialer,
)

listenerEndpoint = newSignerListenerEndpoint(logger, addr, testTimeoutReadWrite)
listenerEndpoint = newSignerListenerEndpoint(logger, l, testTimeoutReadWrite)
)

SignerDialerEndpointTimeoutReadWrite(testTimeoutReadWrite)(dialerEndpoint)
Expand Down
23 changes: 15 additions & 8 deletions pkgs/bft/privval/socket_dialers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package privval

import (
"fmt"
"net"
"testing"
"time"

Expand All @@ -15,34 +16,40 @@ import (
func getDialerTestCases(t *testing.T) []dialerTestCase {
t.Helper()

tcpAddr := GetFreeLocalhostAddrPort()
l, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
panic(fmt.Errorf("no open ports? error listening to 127.0.0.1:0: %w", err))
}
unixFilePath, err := testUnixAddr()
require.NoError(t, err)
unixAddr := fmt.Sprintf("unix://%s", unixFilePath)
ul, err := net.Listen("unix", unixFilePath)
if err != nil {
panic(err)
}

return []dialerTestCase{
{
addr: tcpAddr,
dialer: DialTCPFn(tcpAddr, testTimeoutReadWrite, ed25519.GenPrivKey()),
listener: l,
dialer: DialTCPFn(l.Addr().String(), testTimeoutReadWrite, ed25519.GenPrivKey()),
},
{
addr: unixAddr,
dialer: DialUnixFn(unixFilePath),
listener: ul,
dialer: DialUnixFn(unixFilePath),
},
}
}

func TestIsConnTimeoutForFundamentalTimeouts(t *testing.T) {
// Generate a networking timeout
tcpAddr := GetFreeLocalhostAddrPort()
tcpAddr := "127.0.0.1:34985"
dialer := DialTCPFn(tcpAddr, time.Millisecond, ed25519.GenPrivKey())
_, err := dialer()
assert.Error(t, err)
assert.True(t, IsConnTimeout(err))
}

func TestIsConnTimeoutForWrappedConnTimeouts(t *testing.T) {
tcpAddr := GetFreeLocalhostAddrPort()
tcpAddr := "127.0.0.1:34985"
dialer := DialTCPFn(tcpAddr, time.Millisecond, ed25519.GenPrivKey())
_, err := dialer()
assert.Error(t, err)
Expand Down
9 changes: 0 additions & 9 deletions pkgs/bft/privval/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,3 @@ func NewSignerListener(listenAddr string, logger log.Logger) (*SignerListenerEnd

return pve, nil
}

// GetFreeLocalhostAddrPort returns a free localhost:port address
func GetFreeLocalhostAddrPort() string {
port, err := osm.GetFreePort()
if err != nil {
panic(err)
}
return fmt.Sprintf("127.0.0.1:%d", port)
}
19 changes: 2 additions & 17 deletions pkgs/bft/rpc/test/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
ctypes "github.com/gnolang/gno/pkgs/bft/rpc/core/types"
rpcclient "github.com/gnolang/gno/pkgs/bft/rpc/lib/client"
"github.com/gnolang/gno/pkgs/log"
osm "github.com/gnolang/gno/pkgs/os"
"github.com/gnolang/gno/pkgs/p2p"
)

Expand Down Expand Up @@ -61,27 +60,13 @@ func makePathname() string {
return strings.Replace(p, sep, "_", -1)
}

func randPort() int {
port, err := osm.GetFreePort()
if err != nil {
panic(err)
}
return port
}

func makeAddrs() (string, string) {
return fmt.Sprintf("tcp://0.0.0.0:%d", randPort()),
fmt.Sprintf("tcp://0.0.0.0:%d", randPort())
}

func createConfig() *cfg.Config {
pathname := makePathname()
c := cfg.ResetTestRoot(pathname)

// and we use random ports to run in parallel
tm, rpc := makeAddrs()
c.P2P.ListenAddress = tm
c.RPC.ListenAddress = rpc
c.P2P.ListenAddress = "tcp://127.0.0.2:0"
c.RPC.ListenAddress = "tcp://127.0.0.2:0"
c.RPC.CORSAllowedOrigins = []string{"https://tendermint.com/"}
// c.TxIndex.IndexTags = "app.creator,tx.height" // see kvstore application
return c
Expand Down
17 changes: 0 additions & 17 deletions pkgs/os/net.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,3 @@ func ProtocolAndAddress(listenAddr string) (string, string) {
}
return protocol, address
}

// GetFreePort gets a free port from the operating system.
// Ripped from https://github.com/phayes/freeport.
// BSD-licensed.
func GetFreePort() (int, error) {
addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
if err != nil {
return 0, err
}

l, err := net.ListenTCP("tcp", addr)
if err != nil {
return 0, err
}
defer l.Close()
return l.Addr().(*net.TCPAddr).Port, nil
}
13 changes: 2 additions & 11 deletions pkgs/p2p/test_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/gnolang/gno/pkgs/crypto/ed25519"
"github.com/gnolang/gno/pkgs/errors"
"github.com/gnolang/gno/pkgs/log"
osm "github.com/gnolang/gno/pkgs/os"
"github.com/gnolang/gno/pkgs/p2p/config"
"github.com/gnolang/gno/pkgs/p2p/conn"
"github.com/gnolang/gno/pkgs/random"
Expand Down Expand Up @@ -243,23 +242,15 @@ func testVersionSet() versionset.VersionSet {
func testNodeInfoWithNetwork(id ID, name, network string) NodeInfo {
return NodeInfo{
VersionSet: testVersionSet(),
NetAddress: NewNetAddressFromIPPort(id, net.ParseIP("127.0.0.1"), getFreePort()),
NetAddress: NewNetAddressFromIPPort(id, net.ParseIP("127.0.0.1"), 0),
Network: network,
Software: "p2ptest",
Version: "v1.2.3-rc.0-deadbeef",
Channels: []byte{testCh},
Moniker: name,
Other: NodeInfoOther{
TxIndex: "on",
RPCAddress: fmt.Sprintf("127.0.0.1:%d", getFreePort()),
RPCAddress: fmt.Sprintf("127.0.0.1:%d", 0),
},
}
}

func getFreePort() uint16 {
port, err := osm.GetFreePort()
if err != nil {
panic(err)
}
return uint16(port)
}
12 changes: 12 additions & 0 deletions pkgs/p2p/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net"
"strconv"
"time"

"github.com/gnolang/gno/pkgs/amino"
Expand Down Expand Up @@ -241,6 +242,17 @@ func (mt *MultiplexTransport) Listen(addr NetAddress) error {
return err
}

if addr.Port == 0 {
// net.Listen on port 0 means the kernel will auto-allocate a port
// - find out which one has been given to us.
_, p, err := net.SplitHostPort(ln.Addr().String())
if err != nil {
return fmt.Errorf("error finding port (after listening on port 0): %w", err)
}
pInt, _ := strconv.Atoi(p)
addr.Port = uint16(pInt)
}

mt.netAddr = addr
mt.listener = ln

Expand Down

0 comments on commit 3291945

Please sign in to comment.