fix: race condition

In client.go

    if room, ok := message.(outgoing.Room); ok {
            c.info.RoomID = room.ID
    }

this part isn't thread safe. It could happen that user disconnected but
wasn't removed from a room, because the disconnecting go routine
couldn't see the roomID yet.

ws/client.go

@@ -41,7 +41,6 @@ type ClientMessage struct {
 
 type ClientInfo struct {
 	ID                xid.ID
-	RoomID            string
 	Authenticated     bool
 	AuthenticatedUser string
 	Write             chan outgoing.Message
@@ -60,7 +59,6 @@ func newClient(conn *websocket.Conn, req *http.Request, read chan ClientMessage,
 			Authenticated:     authenticated,
 			AuthenticatedUser: authenticatedUser,
 			ID:                xid.New(),
-			RoomID:            "",
 			Addr:              ip,
 			Write:             make(chan outgoing.Message, 1),
 		},
@@ -158,10 +156,6 @@ func (c *Client) startWriteHandler(pingPeriod time.Duration) {
 				continue
 			}
 
-			if room, ok := message.(outgoing.Room); ok {
-				c.info.RoomID = room.ID
-			}
-
 			if err := writeJSON(c.conn, typed); err != nil {
 				c.printWebSocketError("write", err)
 				c.CloseOnError(websocket.CloseNormalClosure, "write error"+err.Error())

ws/event_clientanswer.go

@@ -16,13 +16,9 @@ func init() {
 type ClientAnswer outgoing.P2PMessage
 
 func (e *ClientAnswer) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	session, ok := room.Sessions[e.SID]

ws/event_clientice.go

@@ -16,13 +16,9 @@ func init() {
 type ClientICE outgoing.P2PMessage
 
 func (e *ClientICE) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	session, ok := room.Sessions[e.SID]

ws/event_connected.go

@@ -3,6 +3,6 @@ package ws
 type Connected struct{}
 
 func (e Connected) Execute(rooms *Rooms, current ClientInfo) error {
-	rooms.connected[current.ID] = true
+	rooms.connected[current.ID] = ""
 	return nil
 }

ws/event_create.go

@@ -23,7 +23,7 @@ type Create struct {
 }
 
 func (e *Create) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID != "" {
+	if rooms.connected[current.ID] != "" {
 		return fmt.Errorf("cannot join room, you are already in one")
 	}
 
@@ -74,6 +74,7 @@ func (e *Create) Execute(rooms *Rooms, current ClientInfo) error {
 			},
 		},
 	}
+	rooms.connected[current.ID] = room.ID
 	rooms.Rooms[e.ID] = room
 	room.notifyInfoChanged()
 	usersJoinedTotal.Inc()

ws/event_disconnected.go

@@ -18,14 +18,15 @@ func (e *Disconnected) Execute(rooms *Rooms, current ClientInfo) error {
 }
 
 func (e *Disconnected) executeNoError(rooms *Rooms, current ClientInfo) {
+	roomID := rooms.connected[current.ID]
 	delete(rooms.connected, current.ID)
 	current.Write <- outgoing.CloseWriter{Code: e.Code, Reason: e.Reason}
 
-	if current.RoomID == "" {
+	if roomID == "" {
 		return
 	}
 
-	room, ok := rooms.Rooms[current.RoomID]
+	room, ok := rooms.Rooms[roomID]
 	if !ok {
 		// room may already be removed
 		return
@@ -63,12 +64,12 @@ func (e *Disconnected) executeNoError(rooms *Rooms, current ClientInfo) {
 			delete(rooms.connected, member.ID)
 			member.Write <- outgoing.CloseWriter{Code: websocket.CloseNormalClosure, Reason: CloseOwnerLeft}
 		}
-		rooms.closeRoom(current.RoomID)
+		rooms.closeRoom(roomID)
 		return
 	}
 
 	if len(room.Users) == 0 {
-		rooms.closeRoom(current.RoomID)
+		rooms.closeRoom(roomID)
 		return
 	}
 

ws/event_hostice.go

@@ -16,13 +16,9 @@ func init() {
 type HostICE outgoing.P2PMessage
 
 func (e *HostICE) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	session, ok := room.Sessions[e.SID]

ws/event_hostoffer.go

@@ -16,13 +16,9 @@ func init() {
 type HostOffer outgoing.P2PMessage
 
 func (e *HostOffer) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	session, ok := room.Sessions[e.SID]

ws/event_join.go

@@ -16,7 +16,7 @@ type Join struct {
 }
 
 func (e *Join) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID != "" {
+	if rooms.connected[current.ID] != "" {
 		return fmt.Errorf("cannot join room, you are already in one")
 	}
 
@@ -40,6 +40,7 @@ func (e *Join) Execute(rooms *Rooms, current ClientInfo) error {
 		Addr:      current.Addr,
 		Write:     current.Write,
 	}
+	rooms.connected[current.ID] = room.ID
 	room.notifyInfoChanged()
 	usersJoinedTotal.Inc()
 

ws/event_name.go

@@ -1,9 +1,5 @@
 package ws
 
-import (
-	"fmt"
-)
-
 func init() {
 	register("name", func() Event {
 		return &Name{}
@@ -15,13 +11,9 @@ type Name struct {
 }
 
 func (e *Name) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	room.Users[current.ID].Name = e.UserName

ws/event_share.go

@@ -1,9 +1,5 @@
 package ws
 
-import (
-	"fmt"
-)
-
 func init() {
 	register("share", func() Event {
 		return &StartShare{}
@@ -13,13 +9,9 @@ func init() {
 type StartShare struct{}
 
 func (e *StartShare) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	room.Users[current.ID].Streaming = true

ws/event_stop_share.go

@@ -2,7 +2,6 @@ package ws
 
 import (
 	"bytes"
-	"fmt"
 
 	"github.com/screego/server/ws/outgoing"
 )
@@ -16,13 +15,9 @@ func init() {
 type StopShare struct{}
 
 func (e *StopShare) Execute(rooms *Rooms, current ClientInfo) error {
-	if current.RoomID == "" {
-		return fmt.Errorf("not in a room")
-	}
-
-	room, ok := rooms.Rooms[current.RoomID]
-	if !ok {
-		return fmt.Errorf("room with id %s does not exist", current.RoomID)
+	room, err := rooms.CurrentRoom(current)
+	if err != nil {
+		return err
 	}
 
 	room.Users[current.ID].Streaming = false

ws/rooms.go

@@ -20,7 +20,7 @@ func NewRooms(tServer turn.Server, users *auth.Users, conf config.Config) *Rooms
 	return &Rooms{
 		Rooms:      map[string]*Room{},
 		Incoming:   make(chan ClientMessage),
-		connected:  map[xid.ID]bool{},
+		connected:  map[xid.ID]string{},
 		turnServer: tServer,
 		users:      users,
 		config:     conf,
@@ -51,7 +51,23 @@ type Rooms struct {
 	users      *auth.Users
 	config     config.Config
 	r          *rand.Rand
-	connected  map[xid.ID]bool
+	connected  map[xid.ID]string
+}
+
+func (r *Rooms) CurrentRoom(info ClientInfo) (*Room, error) {
+	roomID, ok := r.connected[info.ID]
+	if !ok {
+		return nil, fmt.Errorf("not connected")
+	}
+	if roomID == "" {
+		return nil, fmt.Errorf("not in a room")
+	}
+	room, ok := r.Rooms[roomID]
+	if !ok {
+		return nil, fmt.Errorf("room with id %s does not exist", roomID)
+	}
+
+	return room, nil
 }
 
 func (r *Rooms) RandUserName() string {
@@ -81,7 +97,8 @@ func (r *Rooms) Upgrade(w http.ResponseWriter, req *http.Request) {
 
 func (r *Rooms) Start() {
 	for msg := range r.Incoming {
-		if !msg.SkipConnectedCheck && !r.connected[msg.Info.ID] {
+		_, connected := r.connected[msg.Info.ID]
+		if !msg.SkipConnectedCheck && !connected {
 			log.Debug().Interface("event", fmt.Sprintf("%T", msg.Incoming)).Interface("payload", msg.Incoming).Msg("WebSocket Ignore")
 			continue
 		}