From 52b046c9b6a0f6a280ff797f90784f76bfd310b9 Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Wed, 18 Apr 2018 12:27:20 +0200 Subject: [PATCH] rpc: clean up IPC handler (#16524) This avoids logging accept errors on shutdown and removes a bit of duplication. It also fixes some goimports lint warnings. --- cmd/clef/main.go | 6 +++--- common/types.go | 4 ++-- node/node.go | 14 ++------------ rpc/client.go | 2 +- rpc/endpoints.go | 36 +++++++++--------------------------- rpc/ipc.go | 15 ++++++--------- 6 files changed, 23 insertions(+), 54 deletions(-) diff --git a/cmd/clef/main.go b/cmd/clef/main.go index fb91f4c25..9a1ae91ac 100644 --- a/cmd/clef/main.go +++ b/cmd/clef/main.go @@ -23,17 +23,18 @@ import ( "context" "crypto/rand" "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "io" "io/ioutil" "os" + "os/signal" "os/user" "path/filepath" "runtime" "strings" - "encoding/hex" "github.com/ethereum/go-ethereum/cmd/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" @@ -44,7 +45,6 @@ import ( "github.com/ethereum/go-ethereum/signer/rules" "github.com/ethereum/go-ethereum/signer/storage" "gopkg.in/urfave/cli.v1" - "os/signal" ) // ExternalApiVersion -- see extapi_changelog.md @@ -435,7 +435,7 @@ func signer(c *cli.Context) error { ipcApiUrl = filepath.Join(configDir, "clef.ipc") } - listener, _, err := rpc.StartIPCEndpoint(func() bool { return true }, ipcApiUrl, rpcApi) + listener, _, err := rpc.StartIPCEndpoint(ipcApiUrl, rpcApi) if err != nil { utils.Fatalf("Could not start IPC api: %v", err) } diff --git a/common/types.go b/common/types.go index 76e7be58f..0b94fb2c2 100644 --- a/common/types.go +++ b/common/types.go @@ -18,15 +18,15 @@ package common import ( "encoding/hex" + "encoding/json" "fmt" "math/big" "math/rand" "reflect" + "strings" - "encoding/json" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/crypto/sha3" - "strings" ) const ( diff --git a/node/node.go b/node/node.go index bf6e9a7c1..83b6c4c07 100644 --- a/node/node.go +++ b/node/node.go @@ -303,23 +303,13 @@ func (n *Node) stopInProc() { // startIPC initializes and starts the IPC RPC endpoint. func (n *Node) startIPC(apis []rpc.API) error { - // Short circuit if the IPC endpoint isn't being exposed if n.ipcEndpoint == "" { - return nil - + return nil // IPC disabled. } - isClosed := func() bool { - n.lock.RLock() - defer n.lock.RUnlock() - return n.ipcListener == nil - } - - listener, handler, err := rpc.StartIPCEndpoint(isClosed, n.ipcEndpoint, apis) + listener, handler, err := rpc.StartIPCEndpoint(n.ipcEndpoint, apis) if err != nil { return err } - - // All listeners booted successfully n.ipcListener = listener n.ipcHandler = handler n.log.Info("IPC endpoint opened", "url", n.ipcEndpoint) diff --git a/rpc/client.go b/rpc/client.go index 68745c6cb..77b4d5ee0 100644 --- a/rpc/client.go +++ b/rpc/client.go @@ -25,6 +25,7 @@ import ( "fmt" "net" "net/url" + "os" "reflect" "strconv" "strings" @@ -33,7 +34,6 @@ import ( "time" "github.com/ethereum/go-ethereum/log" - "os" ) var ( diff --git a/rpc/endpoints.go b/rpc/endpoints.go index 9ba2ed970..692c62d3a 100644 --- a/rpc/endpoints.go +++ b/rpc/endpoints.go @@ -17,8 +17,9 @@ package rpc import ( - "github.com/ethereum/go-ethereum/log" "net" + + "github.com/ethereum/go-ethereum/log" ) // StartHTTPEndpoint starts the HTTP RPC endpoint, configured with cors/vhosts/modules @@ -81,9 +82,9 @@ func StartWSEndpoint(endpoint string, apis []API, modules []string, wsOrigins [] } -// StartIPCEndpoint starts an IPC endpoint -func StartIPCEndpoint(isClosedFn func() bool, ipcEndpoint string, apis []API) (net.Listener, *Server, error) { - // Register all the APIs exposed by the services +// StartIPCEndpoint starts an IPC endpoint. +func StartIPCEndpoint(ipcEndpoint string, apis []API) (net.Listener, *Server, error) { + // Register all the APIs exposed by the services. handler := NewServer() for _, api := range apis { if err := handler.RegisterName(api.Namespace, api.Service); err != nil { @@ -91,30 +92,11 @@ func StartIPCEndpoint(isClosedFn func() bool, ipcEndpoint string, apis []API) (n } log.Debug("IPC registered", "namespace", api.Namespace) } - // All APIs registered, start the IPC listener - var ( - listener net.Listener - err error - ) - if listener, err = CreateIPCListener(ipcEndpoint); err != nil { + // All APIs registered, start the IPC listener. + listener, err := ipcListen(ipcEndpoint) + if err != nil { return nil, nil, err } - go func() { - for { - conn, err := listener.Accept() - if err != nil { - // Terminate if the listener was closed - if isClosedFn() { - log.Info("IPC closed", "err", err) - } else { - // Not closed, just some error; report and continue - log.Error("IPC accept failed", "err", err) - } - continue - } - go handler.ServeCodec(NewJSONCodec(conn), OptionMethodInvocation|OptionSubscriptions) - } - }() - + go handler.ServeListener(listener) return listener, handler, nil } diff --git a/rpc/ipc.go b/rpc/ipc.go index 8de18a56f..b05e503d7 100644 --- a/rpc/ipc.go +++ b/rpc/ipc.go @@ -18,26 +18,23 @@ package rpc import ( "context" - "fmt" "net" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/p2p/netutil" ) -// CreateIPCListener creates an listener, on Unix platforms this is a unix socket, on -// Windows this is a named pipe -func CreateIPCListener(endpoint string) (net.Listener, error) { - return ipcListen(endpoint) -} - // ServeListener accepts connections on l, serving JSON-RPC on them. func (srv *Server) ServeListener(l net.Listener) error { for { conn, err := l.Accept() - if err != nil { + if netutil.IsTemporaryError(err) { + log.Warn("RPC accept error", "err", err) + continue + } else if err != nil { return err } - log.Trace(fmt.Sprint("accepted conn", conn.RemoteAddr())) + log.Trace("Accepted connection", "addr", conn.RemoteAddr()) go srv.ServeCodec(NewJSONCodec(conn), OptionMethodInvocation|OptionSubscriptions) } }