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.

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
-
-	}
-	isClosed := func() bool {
-		n.lock.RLock()
-		defer n.lock.RUnlock()
-		return n.ipcListener == nil
+		return nil // IPC disabled.
 	}
-
-	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)
 	}
 }