Skip to content

Commit

Permalink
bugfix(client): Fix the panic due to concurrent writing (#180)
Browse files Browse the repository at this point in the history
The bug:
	panic: concurrent write to websocket connection
	goroutine 66624 [running]:
	github.com/gorilla/websocket.(*messageWriter).flushFrame(0xc001ad7ef0, 0x1, {0x0?, 0x0?, 0x0?})
		/home/streampanel/go/pkg/mod/github.com/gorilla/[email protected]/conn.go:617 +0x4b8
	github.com/gorilla/websocket.(*messageWriter).Close(0xc001e12f60?)
		/home/streampanel/go/pkg/mod/github.com/gorilla/[email protected]/conn.go:731 +0x35
	github.com/gorilla/websocket.(*Conn).beginMessage(0xc001e034a0, 0xc001e12f60, 0x8)
		/home/streampanel/go/pkg/mod/github.com/gorilla/[email protected]/conn.go:480 +0x3a
	github.com/gorilla/websocket.(*Conn).NextWriter(0xc001e034a0, 0x8)
		/home/streampanel/go/pkg/mod/github.com/gorilla/[email protected]/conn.go:520 +0x3f
	github.com/gorilla/websocket.(*Conn).WriteMessage(0x37d2cb0?, 0x1e5f71c?, {0xc000945490, 0x5, 0x5})
		/home/streampanel/go/pkg/mod/github.com/gorilla/[email protected]/conn.go:773 +0x137
	github.com/andreykaipov/goobs.(*Client).Disconnect(0xc001c26fc0)
		/home/streampanel/go/pkg/mod/github.com/xaionaro-go/[email protected]/client.go:123 +0xe5

The documentation of github.com/gorilla/websocket explicitly forbids concurrent writing and reading.
See section "Concurrency" in:

	https://pkg.go.dev/github.com/gorilla/websocket#hdr-Concurrency
  • Loading branch information
xaionaro authored Oct 25, 2024
1 parent bd2c39d commit 3c6c796
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type Client struct {
Categories

conn *websocket.Conn
connWLocker sync.Mutex
connRLocker sync.Mutex
scheme string
host string
password string
Expand Down Expand Up @@ -120,7 +122,7 @@ func (c *Client) Disconnect() error {
c.client.Log.Printf("[DEBUG] Sending disconnect message")
c.markDisconnected()

if err := c.conn.WriteMessage(
if err := c.writeMessage(
websocket.CloseMessage,
websocket.FormatCloseMessage(websocket.CloseNormalClosure, "Bye"),
); err != nil {
Expand All @@ -131,6 +133,15 @@ func (c *Client) Disconnect() error {
return nil
}

func (c *Client) writeMessage(messageType int, data []byte) error {
c.connWLocker.Lock()
defer c.connWLocker.Unlock()
return c.conn.WriteMessage(
messageType,
data,
)
}

func (c *Client) markDisconnected() {
c.once.Do(func() {
select {
Expand Down Expand Up @@ -222,13 +233,19 @@ func (c *Client) connect() (err error) {
}
}

func (c *Client) readJSON(v any) error {
c.connRLocker.Lock()
defer c.connRLocker.Unlock()
return c.conn.ReadJSON(v)
}

// translates raw server messages into opcodes
func (c *Client) handleRawServerMessages(auth chan<- error) {
defer c.client.Log.Printf("[TRACE] Finished handling raw server messages")

for {
raw := json.RawMessage{}
if err := c.conn.ReadJSON(&raw); err != nil {
if err := c.readJSON(&raw); err != nil {
switch t := err.(type) {
case *json.UnmarshalTypeError:
c.client.Log.Printf("[ERROR] Reading from connection: %s: %s", t, raw)
Expand Down Expand Up @@ -323,7 +340,7 @@ func (c *Client) handleOpcodes(auth chan<- error) {
c.client.Log.Printf("[INFO] Identify;")

msg := opcodes.Wrap(val).Bytes()
if err := c.conn.WriteMessage(websocket.TextMessage, msg); err != nil {
if err := c.writeMessage(websocket.TextMessage, msg); err != nil {
auth <- fmt.Errorf("sending Identify to server `%s`: %w", msg, err)
}

Expand Down Expand Up @@ -354,7 +371,7 @@ func (c *Client) handleOpcodes(auth chan<- error) {
c.client.Log.Printf("[TRACE] Got %s Request with ID %s", val.Type, val.ID)

msg := opcodes.Wrap(val).Bytes()
if err := c.conn.WriteMessage(websocket.TextMessage, msg); err != nil {
if err := c.writeMessage(websocket.TextMessage, msg); err != nil {
c.client.Log.Printf("[ERROR] Sending Request to server `%s`: %s", msg, err)
}

Expand Down

0 comments on commit 3c6c796

Please sign in to comment.