Skip to content

Commit

Permalink
ethstats: split read and write lock, otherwise they'll lock up
Browse files Browse the repository at this point in the history
  • Loading branch information
karalabe authored and mandrigin committed Aug 14, 2020
1 parent 34a708e commit 9248f48
Showing 1 changed file with 18 additions and 14 deletions.
32 changes: 18 additions & 14 deletions ethstats/ethstats.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,20 @@ type Service struct {

// connWrapper is a wrapper to prevent concurrent-write or concurrent-read on the
// websocket.
// From Gorilla websocket docs:
// Connections support one concurrent reader and one concurrent writer.
// Applications are responsible for ensuring that no more than one goroutine calls the write methods
// - NextWriter, SetWriteDeadline, WriteMessage, WriteJSON, EnableWriteCompression, SetCompressionLevel
// concurrently and that no more than one goroutine calls the read methods
// - NextReader, SetReadDeadline, ReadMessage, ReadJSON, SetPongHandler, SetPingHandler
// concurrently.
// The Close and WriteControl methods can be called concurrently with all other methods.
//
// The connWrapper uses a single mutex for both reading and writing.
// From Gorilla websocket docs:
// Connections support one concurrent reader and one concurrent writer.
// Applications are responsible for ensuring that no more than one goroutine calls the write methods
// - NextWriter, SetWriteDeadline, WriteMessage, WriteJSON, EnableWriteCompression, SetCompressionLevel
// concurrently and that no more than one goroutine calls the read methods
// - NextReader, SetReadDeadline, ReadMessage, ReadJSON, SetPongHandler, SetPingHandler
// concurrently.
// The Close and WriteControl methods can be called concurrently with all other methods.
type connWrapper struct {
conn *websocket.Conn
mu sync.Mutex

rlock sync.Mutex
wlock sync.Mutex
}

func newConnectionWrapper(conn *websocket.Conn) *connWrapper {
Expand All @@ -116,15 +117,17 @@ func newConnectionWrapper(conn *websocket.Conn) *connWrapper {

// WriteJSON wraps corresponding method on the websocket but is safe for concurrent calling
func (w *connWrapper) WriteJSON(v interface{}) error {
w.mu.Lock()
defer w.mu.Unlock()
w.wlock.Lock()
defer w.wlock.Unlock()

return w.conn.WriteJSON(v)
}

// ReadJSON wraps corresponding method on the websocket but is safe for concurrent calling
func (w *connWrapper) ReadJSON(v interface{}) error {
w.mu.Lock()
defer w.mu.Unlock()
w.rlock.Lock()
defer w.rlock.Unlock()

return w.conn.ReadJSON(v)
}

Expand Down Expand Up @@ -271,6 +274,7 @@ func (s *Service) loop() {
continue
}
go s.readLoop(conn)

// Send the initial stats so our node looks decent from the get go
if err = s.report(conn); err != nil {
log.Warn("Initial stats report failed", "err", err)
Expand Down

0 comments on commit 9248f48

Please sign in to comment.