ethereum / go-ethereum

Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
47.28k stars 20.02k forks source link

RPC Client doesn't gracefully close Websocket Connections #30482

Open moorow opened 1 week ago

moorow commented 1 week ago

System information

Geth version: 1.14.8 OS & Version: OSX

Expected behaviour

When rpc.Client Close() is called, I would expect a message like this to be sent

err := cli.writeMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""))

Actual behaviour

The client closes the TCP conn without sending a WS Close message, leading to websocket: close 1006 (abnormal closure): unexpected EOF on the server side.

Steps to reproduce the behaviour

Open a WS connection with the rpc.Client. Close the connection.

holiman commented 1 week ago

Odd. In rpc/client.go, the dispatch loop calls close on the underlying conn

    defer func() {
        close(c.closing)
        if reading {
            conn.close(ErrClientQuit, nil)
            c.drainRead()
        }
        close(c.didClose)
    }()

At some point, I'd have expected that to trickle down to the gorilla/websocket conn, which has the following default close-handler:

func (c *Conn) SetCloseHandler(h func(code int, text string) error) {
    if h == nil {
        h = func(code int, text string) error {
            message := FormatCloseMessage(code, "")
            c.WriteControl(CloseMessage, message, time.Now().Add(writeWait))
            return nil
        }
    }
    c.handleClose = h
}
moorow commented 1 week ago

So conn.close(ErrClientQuit, nil) eventually calls close on the jsonCodec

func (c *jsonCodec) close() {
    c.closer.Do(func() {
        close(c.closeCh)
        c.conn.Close()
    })
}

This in turn calls Close on the Gorrillas WS conn as you say.

// Close closes the underlying network connection without sending or waiting
// for a close message.
func (c *Conn) Close() error {
    return c.conn.Close()
}

This function unfortunately just closes the TPC connection.

The Close Handler only gets called if a CloseMessage is received.