gorilla/websocket

concurrent write to websocket connection

plum330 opened this issue · 4 comments

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

concurrent write to websocket connection

Expected Behavior

normal write

Steps To Reproduce

Describe the problem you're having

Using the gorilla/websocket package I sometimes get the following error, despite using mutex.Lock:
concurrent write to websocket connection

Versions

Go version: 1.19
package version: v1.5.1
"Show me the code!"
// goroutine
func (s *Session) write() {
tk := time.NewTicker(interval * 4 / 5)
defer func() {
if e := recover(); e != nil {
// TODO
}
tk.Stop()
s.Close()
}()

for {
	if s.closed.Load() {
		return
	}
	select {
	case m := <-s.out:
		s.conn.SetWriteDeadline(time.Now().Add(wTime))
		err := s.conn.WriteMessage(m.Type, m.Payload)
		if err != nil {
			return
		}
	case <-tk.C:
		s.conn.SetWriteDeadline(time.Now().Add(wTime))
		err := s.conn.WriteControl(websocket.PingMessage, []byte{}, time.Now().Add(wTime))
		if err != nil {
			return
		}
	}
}

}

// goroutine
func (s *Session) handleHB() {
s.conn.SetPongHandler(func(msg string) error {
_ = s.conn.SetReadDeadline(time.Now().Add(interval))
atomic.StoreInt64(&s.hbTime, time.Now().Unix())
return nil
})
s.conn.SetPingHandler(func(msg string) error {
_ = s.conn.SetReadDeadline(time.Now().Add(interval))
atomic.StoreInt64(&s.hbTime, time.Now().Unix())
return s.conn.WriteControl(websocket.PongMessage, []byte{}, time.Now().Add(wTime))
})
s.conn.SetCloseHandler(func(code int, text string) error {
return nil
})

for {
	ts := atomic.LoadInt64(&s.hbTime)
	if time.Now().Unix()-ts > int64(s.opt.hbInterval.Seconds()) {
		return
	}
	time.Sleep(interval * 1 / 10)
}

}
from #652 (comment) to know WriteControl and WriteMessage can use at the same time. So, What can cause concurrent write problems.

Anything else?

No response

Hey @go-slark,

Was browsing and saw your issue. Ensure that you're using a mutex for your write operations to prevent any overlap. Kind of like this:

func (s *Session) write() {
   ...
   for {
      ...
      s.writeMutex.Lock()
      select {
      case m := <-s.out:
         ...
         s.writeMutex.Unlock()
         ...
      case <-tk.C:
         ...
         s.writeMutex.Unlock()
         ...
      }
   }
}

The same problem.
When I run my code with the race flag go run -race ./main.go I get a message:

WARNING: DATA RACE
Read at 0x00c00020e078 by goroutine 7:
github.com/gorilla/websocket.(*Conn).beginMessage()
/home/yash/go/pkg/mod/github.com/gorilla/websocket@v1.5.1/conn.go:492 +0x5c
github.com/gorilla/websocket.(*Conn).WriteMessage()
/home/yash/go/pkg/mod/github.com/gorilla/websocket@v1.5.1/conn.go:779 +0xeb
.....

Previous write at 0x00c00020e078 by goroutine 10:
github.com/gorilla/websocket.(*messageWriter).endMessage()
/home/yash/go/pkg/mod/github.com/gorilla/websocket@v1.5.1/conn.go:561 +0xba
github.com/gorilla/websocket.(*messageWriter).flushFrame()
/home/yash/go/pkg/mod/github.com/gorilla/websocket@v1.5.1/conn.go:648 +0xb84
github.com/gorilla/websocket.(*messageWriter).Close()
/home/yash/go/pkg/mod/github.com/gorilla/websocket@v1.5.1/conn.go:746 +0x8f
github.com/gorilla/websocket.(*Conn).WriteJSON()
/home/yash/go/pkg/mod/github.com/gorilla/websocket@v1.5.1/json.go:29 +0x15e
.....

@yyasha Look at the callers of WriteMessage and WriteJSON (not shown in your comment) and ensure that those callers do not allow concurrent calls to the write methods.