Various race preventing locks

client.go

@@ -52,7 +52,7 @@ type Client struct {
 	sync.Mutex
 }
 
-func (c Client) Host() string {
+func (c *Client) Host() string {
 	addr := c.conn.RemoteAddr().String()
 	if host, _, err := net.SplitHostPort(addr); err == nil {
 		addr = host
@@ -63,7 +63,7 @@ func (c Client) Host() string {
 	return addr
 }
 
-func (c Client) String() string {
+func (c *Client) String() string {
 	return *c.nickname + "!" + *c.username + "@" + c.Host()
 }
 

daemon.go

@@ -40,18 +40,23 @@ const (
 var (
 	RENickname = regexp.MustCompile("^[a-zA-Z0-9-]{1,24}$")
 
-	roomsGroup sync.WaitGroup
-
 	clients    map[*Client]struct{} = make(map[*Client]struct{})
+	clientsM   sync.RWMutex
+	rooms      map[string]*Room = make(map[string]*Room)
+	roomsM     sync.RWMutex
+	roomsGroup sync.WaitGroup
+	roomSinks  map[*Room]chan ClientEvent = make(map[*Room]chan ClientEvent)
 )
 
 func SendLusers(client *Client) {
 	lusers := 0
+	clientsM.RLock()
 	for client := range clients {
 		if client.registered {
 			lusers++
 		}
 	}
+	clientsM.RUnlock()
 	client.ReplyNicknamed("251", fmt.Sprintf("There are %d users and 0 invisible on 1 servers", lusers))
 }
 
@@ -82,11 +87,14 @@ func SendWhois(client *Client, nicknames []string) {
 	var subscriber *Client
 	for _, nickname := range nicknames {
 		nickname = strings.ToLower(nickname)
+		clientsM.RLock()
 		for c = range clients {
 			if strings.ToLower(*c.nickname) == nickname {
+				clientsM.RUnlock()
 				goto Found
 			}
 		}
+		clientsM.RUnlock()
 		client.ReplyNoNickChan(nickname)
 		continue
 	Found:
@@ -101,6 +109,7 @@ func SendWhois(client *Client, nicknames []string) {
 			client.ReplyNicknamed("301", *c.nickname, *c.away)
 		}
 		subscriptions = make([]string, 0)
+		roomsM.RLock()
 		for _, room = range rooms {
 			for subscriber = range room.members {
 				if *subscriber.nickname == nickname {
@@ -108,6 +117,7 @@ func SendWhois(client *Client, nicknames []string) {
 				}
 			}
 		}
+		roomsM.RUnlock()
 		sort.Strings(subscriptions)
 		client.ReplyNicknamed("319", *c.nickname, strings.Join(subscriptions, " "))
 		client.ReplyNicknamed("318", *c.nickname, "End of /WHOIS list")
@@ -121,14 +131,17 @@ func SendList(client *Client, cols []string) {
 		rs = strings.Split(strings.Split(cols[1], " ")[0], ",")
 	} else {
 		rs = make([]string, 0)
+		roomsM.RLock()
 		for r = range rooms {
 			rs = append(rs, r)
 		}
+		roomsM.RUnlock()
 	}
 	sort.Strings(rs)
 	var room *Room
 	var found bool
 	for _, r = range rs {
+		roomsM.RLock()
 		if room, found = rooms[r]; found {
 			client.ReplyNicknamed(
 				"322",
@@ -137,6 +150,7 @@ func SendList(client *Client, cols []string) {
 				*room.topic,
 			)
 		}
+		roomsM.RUnlock()
 	}
 	client.ReplyNicknamed("323", "End of /LIST")
 }
@@ -163,12 +177,15 @@ func ClientRegister(client *Client, cmd string, cols []string) {
 		// Compatibility with some clients prepending colons to nickname
 		nickname = strings.TrimPrefix(nickname, ":")
 		nickname = strings.ToLower(nickname)
+		clientsM.RLock()
 		for existingClient := range clients {
 			if *existingClient.nickname == nickname {
+				clientsM.RUnlock()
 				client.ReplyParts("433", "*", nickname, "Nickname is already in use")
 				return
 			}
 		}
+		clientsM.RUnlock()
 		if !RENickname.MatchString(nickname) {
 			client.ReplyParts("432", "*", cols[1], "Erroneous nickname")
 			return
@@ -227,8 +244,10 @@ func ClientRegister(client *Client, cmd string, cols []string) {
 func RoomRegister(name string) (*Room, chan ClientEvent) {
 	roomNew := NewRoom(name)
 	roomSink := make(chan ClientEvent)
+	roomsM.Lock()
 	rooms[name] = roomNew
 	roomSinks[roomNew] = roomSink
+	roomsM.Unlock()
 	go roomNew.Processor(roomSink)
 	roomsGroup.Add(1)
 	return roomNew, roomSink
@@ -257,8 +276,10 @@ func HandlerJoin(client *Client, cmd string) {
 		} else {
 			key = ""
 		}
+		roomsM.RLock()
 		for roomExisting, roomSink = range roomSinks {
 			if room == *roomExisting.name {
+				roomsM.RUnlock()
 				if (*roomExisting.key != "") && (*roomExisting.key != key) {
 					goto Denied
 				}
@@ -266,6 +287,7 @@ func HandlerJoin(client *Client, cmd string) {
 				goto Joined
 			}
 		}
+		roomsM.RUnlock()
 		roomNew, roomSink = RoomRegister(room)
 		log.Println("Room", roomNew, "created")
 		if key != "" {
@@ -293,6 +315,7 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 		client := event.client
 		switch event.eventType {
 		case EventTick:
+			clientsM.RLock()
 			for c := range clients {
 				if c.recvTimestamp.Add(PingTimeout).Before(now) {
 					log.Println(c, "ping timeout")
@@ -309,6 +332,8 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 					}
 				}
 			}
+			clientsM.RUnlock()
+			roomsM.Lock()
 			for rn, r := range rooms {
 				if *statedir == "" && len(r.members) == 0 {
 					log.Println(rn, "emptied room")
@@ -317,20 +342,29 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 					delete(roomSinks, r)
 				}
 			}
+			roomsM.Unlock()
 		case EventTerm:
+			roomsM.RLock()
 			for _, sink := range roomSinks {
 				sink <- ClientEvent{eventType: EventTerm}
 			}
+			roomsM.RUnlock()
 			roomsGroup.Wait()
 			close(finished)
 			return
 		case EventNew:
+			clientsM.Lock()
 			clients[client] = struct{}{}
+			clientsM.Unlock()
 		case EventDel:
+			clientsM.Lock()
 			delete(clients, client)
+			clientsM.Unlock()
+			roomsM.RLock()
 			for _, roomSink := range roomSinks {
 				roomSink <- event
 			}
+			roomsM.RUnlock()
 		case EventMsg:
 			cols := strings.SplitN(event.text, " ", 2)
 			cmd := strings.ToUpper(cols[0])
@@ -384,9 +418,11 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 					continue
 				}
 				room := cols[0]
+				roomsM.RLock()
 				r, found := rooms[room]
 				if !found {
 					client.ReplyNoChannel(room)
+					roomsM.RUnlock()
 					continue
 				}
 				if len(cols) == 1 {
@@ -394,6 +430,7 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 				} else {
 					roomSinks[r] <- ClientEvent{client, EventMode, cols[1]}
 				}
+				roomsM.RUnlock()
 			case "MOTD":
 				SendMotd(client)
 			case "PART":
@@ -402,14 +439,17 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 					continue
 				}
 				rs := strings.Split(cols[1], " ")[0]
+				roomsM.RLock()
 				for _, room := range strings.Split(rs, ",") {
 					if r, found := rooms[room]; found {
 						roomSinks[r] <- ClientEvent{client, EventDel, ""}
 					} else {
+						roomsM.RUnlock()
 						client.ReplyNoChannel(room)
 						continue
 					}
 				}
+				roomsM.RUnlock()
 			case "PING":
 				if len(cols) == 1 {
 					client.ReplyNicknamed("409", "No origin specified")
@@ -430,6 +470,7 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 				}
 				msg := ""
 				target := strings.ToLower(cols[0])
+				clientsM.RLock()
 				for c := range clients {
 					if *c.nickname == target {
 						msg = fmt.Sprintf(":%s %s %s %s", client, cmd, *c.nickname, cols[1])
@@ -440,9 +481,11 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 						break
 					}
 				}
+				clientsM.RUnlock()
 				if msg != "" {
 					continue
 				}
+				roomsM.RLock()
 				if r, found := rooms[target]; found {
 					roomSinks[r] <- ClientEvent{
 						client,
@@ -452,13 +495,16 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 				} else {
 					client.ReplyNoNickChan(target)
 				}
+				roomsM.RUnlock()
 			case "TOPIC":
 				if len(cols) == 1 {
 					client.ReplyNotEnoughParameters("TOPIC")
 					continue
 				}
 				cols = strings.SplitN(cols[1], " ", 2)
+				roomsM.RLock()
 				r, found := rooms[cols[0]]
+				roomsM.RUnlock()
 				if !found {
 					client.ReplyNoChannel(cols[0])
 					continue
@@ -469,18 +515,22 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 				} else {
 					change = ""
 				}
+				roomsM.RLock()
 				roomSinks[r] <- ClientEvent{client, EventTopic, change}
+				roomsM.RUnlock()
 			case "WHO":
 				if len(cols) == 1 || len(cols[1]) < 1 {
 					client.ReplyNotEnoughParameters("WHO")
 					continue
 				}
 				room := strings.Split(cols[1], " ")[0]
+				roomsM.RLock()
 				if r, found := rooms[room]; found {
 					roomSinks[r] <- ClientEvent{client, EventWho, ""}
 				} else {
 					client.ReplyNoChannel(room)
 				}
+				roomsM.RUnlock()
 			case "WHOIS":
 				if len(cols) == 1 || len(cols[1]) < 1 {
 					client.ReplyNotEnoughParameters("WHOIS")
@@ -495,9 +545,11 @@ func Processor(events chan ClientEvent, finished chan struct{}) {
 					continue
 				}
 				nicksKnown := make(map[string]struct{})
+				clientsM.RLock()
 				for c := range clients {
 					nicksKnown[*c.nickname] = struct{}{}
 				}
+				clientsM.RUnlock()
 				var nicksExists []string
 				for _, nickname := range strings.Split(cols[1], " ") {
 					if _, exists := nicksKnown[nickname]; exists {

room.go

@@ -24,14 +24,11 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"sync"
 )
 
 var (
 	RERoom = regexp.MustCompile("^#[^\x00\x07\x0a\x0d ,:/]{1,200}$")
-
-	rooms map[string]*Room = make(map[string]*Room)
-
-	roomSinks map[*Room]chan ClientEvent = make(map[*Room]chan ClientEvent)
 )
 
 // Sanitize room's name. It can consist of 1 to 50 ASCII symbols
@@ -45,10 +42,14 @@ type Room struct {
 	topic   *string
 	key     *string
 	members map[*Client]struct{}
+	sync.RWMutex
 }
 
-func (room Room) String() string {
+func (room *Room) String() (name string) {
-	return *room.name
+	room.RLock()
+	name = *room.name
+	room.RUnlock()
+	return
 }
 
 func NewRoom(name string) *Room {
@@ -63,25 +64,31 @@ func NewRoom(name string) *Room {
 }
 
 func (room *Room) SendTopic(client *Client) {
+	room.RLock()
 	if *room.topic == "" {
-		client.ReplyNicknamed("331", *room.name, "No topic is set")
+		client.ReplyNicknamed("331", room.String(), "No topic is set")
 	} else {
-		client.ReplyNicknamed("332", *room.name, *room.topic)
+		client.ReplyNicknamed("332", room.String(), *room.topic)
 	}
+	room.RUnlock()
 }
 
 // Send message to all room's subscribers, possibly excluding someone.
 func (room *Room) Broadcast(msg string, clientToIgnore ...*Client) {
+	room.RLock()
 	for member := range room.members {
 		if (len(clientToIgnore) > 0) && member == clientToIgnore[0] {
 			continue
 		}
 		member.Msg(msg)
 	}
+	room.RUnlock()
 }
 
 func (room *Room) StateSave() {
-	stateSink <- StateEvent{*room.name, *room.topic, *room.key}
+	room.RLock()
+	stateSink <- StateEvent{room.String(), *room.topic, *room.key}
+	room.RUnlock()
 }
 
 func (room *Room) Processor(events <-chan ClientEvent) {
@@ -93,54 +100,74 @@ func (room *Room) Processor(events <-chan ClientEvent) {
 			roomsGroup.Done()
 			return
 		case EventNew:
+			room.Lock()
 			room.members[client] = struct{}{}
 			if *verbose {
 				log.Println(client, "joined", room.name)
 			}
+			room.Unlock()
 			room.SendTopic(client)
-			room.Broadcast(fmt.Sprintf(":%s JOIN %s", client, *room.name))
+			room.Broadcast(fmt.Sprintf(":%s JOIN %s", client, room.String()))
-			logSink <- LogEvent{*room.name, *client.nickname, "joined", true}
+			logSink <- LogEvent{room.String(), *client.nickname, "joined", true}
 			nicknames := make([]string, 0)
+			room.RLock()
 			for member := range room.members {
 				nicknames = append(nicknames, *member.nickname)
 			}
+			room.RUnlock()
 			sort.Strings(nicknames)
-			client.ReplyNicknamed("353", "=", *room.name, strings.Join(nicknames, " "))
+			client.ReplyNicknamed("353", "=", room.String(), strings.Join(nicknames, " "))
-			client.ReplyNicknamed("366", *room.name, "End of NAMES list")
+			client.ReplyNicknamed("366", room.String(), "End of NAMES list")
 		case EventDel:
+			room.RLock()
 			if _, subscribed := room.members[client]; !subscribed {
-				client.ReplyNicknamed("442", *room.name, "You are not on that channel")
+				client.ReplyNicknamed("442", room.String(), "You are not on that channel")
+				room.RUnlock()
 				continue
 			}
+			room.RUnlock()
+			room.Lock()
 			delete(room.members, client)
-			msg := fmt.Sprintf(":%s PART %s :%s", client, *room.name, *client.nickname)
+			room.Unlock()
+			room.RLock()
+			msg := fmt.Sprintf(":%s PART %s :%s", client, room.String(), *client.nickname)
 			room.Broadcast(msg)
-			logSink <- LogEvent{*room.name, *client.nickname, "left", true}
+			logSink <- LogEvent{room.String(), *client.nickname, "left", true}
+			room.RUnlock()
 		case EventTopic:
+			room.RLock()
 			if _, subscribed := room.members[client]; !subscribed {
-				client.ReplyParts("442", *room.name, "You are not on that channel")
+				client.ReplyParts("442", room.String(), "You are not on that channel")
+				room.RUnlock()
 				continue
 			}
 			if event.text == "" {
 				room.SendTopic(client)
+				room.RUnlock()
 				continue
 			}
+			room.RUnlock()
 			topic := strings.TrimLeft(event.text, ":")
+			room.Lock()
 			room.topic = &topic
-			msg := fmt.Sprintf(":%s TOPIC %s :%s", client, *room.name, *room.topic)
+			room.Unlock()
+			room.RLock()
+			msg := fmt.Sprintf(":%s TOPIC %s :%s", client, room.String(), *room.topic)
 			room.Broadcast(msg)
 			logSink <- LogEvent{
-				*room.name,
+				room.String(),
 				*client.nickname,
 				"set topic to " + *room.topic,
 				true,
 			}
+			room.RUnlock()
 			room.StateSave()
 		case EventWho:
+			room.RLock()
 			for m := range room.members {
 				client.ReplyNicknamed(
 					"352",
-					*room.name,
+					room.String(),
 					*m.username,
 					m.Host(),
 					*hostname,
@@ -149,29 +176,36 @@ func (room *Room) Processor(events <-chan ClientEvent) {
 					"0 "+*m.realname,
 				)
 			}
-			client.ReplyNicknamed("315", *room.name, "End of /WHO list")
+			client.ReplyNicknamed("315", room.String(), "End of /WHO list")
+			room.RUnlock()
 		case EventMode:
+			room.RLock()
 			if event.text == "" {
 				mode := "+"
 				if *room.key != "" {
 					mode = mode + "k"
 				}
-				client.Msg(fmt.Sprintf("324 %s %s %s", *client.nickname, *room.name, mode))
+				client.Msg(fmt.Sprintf("324 %s %s %s", *client.nickname, room.String(), mode))
+				room.RUnlock()
 				continue
 			}
 			if strings.HasPrefix(event.text, "b") {
-				client.ReplyNicknamed("368", *room.name, "End of channel ban list")
+				client.ReplyNicknamed("368", room.String(), "End of channel ban list")
+				room.RUnlock()
 				continue
 			}
 			if strings.HasPrefix(event.text, "-k") || strings.HasPrefix(event.text, "+k") {
 				if _, subscribed := room.members[client]; !subscribed {
-					client.ReplyParts("442", *room.name, "You are not on that channel")
+					client.ReplyParts("442", room.String(), "You are not on that channel")
+					room.RUnlock()
 					continue
 				}
 			} else {
 				client.ReplyNicknamed("472", event.text, "Unknown MODE flag")
+				room.RUnlock()
 				continue
 			}
+			room.RUnlock()
 			var msg string
 			var msgLog string
 			if strings.HasPrefix(event.text, "+k") {
@@ -180,17 +214,21 @@ func (room *Room) Processor(events <-chan ClientEvent) {
 					client.ReplyNotEnoughParameters("MODE")
 					continue
 				}
+				room.Lock()
 				room.key = &cols[1]
 				msg = fmt.Sprintf(":%s MODE %s +k %s", client, *room.name, *room.key)
 				msgLog = "set channel key to " + *room.key
+				room.Unlock()
 			} else {
 				key := ""
+				room.Lock()
 				room.key = &key
 				msg = fmt.Sprintf(":%s MODE %s -k", client, *room.name)
+				room.Unlock()
 				msgLog = "removed channel key"
 			}
 			room.Broadcast(msg)
-			logSink <- LogEvent{*room.name, *client.nickname, msgLog, true}
+			logSink <- LogEvent{room.String(), *client.nickname, msgLog, true}
 			room.StateSave()
 		case EventMsg:
 			sep := strings.Index(event.text, " ")
@@ -198,12 +236,12 @@ func (room *Room) Processor(events <-chan ClientEvent) {
 				":%s %s %s :%s",
 				client,
 				event.text[:sep],
-				*room.name,
+				room.String(),
 				event.text[sep+1:]),
 				client,
 			)
 			logSink <- LogEvent{
-				*room.name,
+				room.String(),
 				*client.nickname,
 				event.text[sep+1:],
 				false,

room_test.go

@@ -47,9 +47,11 @@ func TestTwoUsers(t *testing.T) {
 	host := "foohost"
 	hostname = &host
 	events := make(chan ClientEvent)
+	roomsM.Lock()
 	rooms = make(map[string]*Room)
-	clients = make(map[*Client]struct{})
 	roomSinks = make(map[*Room]chan ClientEvent)
+	roomsM.Unlock()
+	clients = make(map[*Client]struct{})
 	finished := make(chan struct{})
 	go Processor(events, finished)
 	defer func() {
@@ -182,12 +184,14 @@ func TestJoin(t *testing.T) {
 	for i := 0; i < 4*2; i++ {
 		<-conn.outbound
 	}
+	roomsM.RLock()
 	if _, ok := rooms["#bar"]; !ok {
 		t.Fatal("#bar does not exist")
 	}
 	if _, ok := rooms["#baz"]; !ok {
 		t.Fatal("#baz does not exist")
 	}
+	roomsM.RUnlock()
 	if r := <-logSink; (r.what != "joined") || (r.where != "#bar") || (r.who != "nick2") || (r.meta != true) {
 		t.Fatal("invalid join log event #bar", r)
 	}
@@ -199,12 +203,14 @@ func TestJoin(t *testing.T) {
 	for i := 0; i < 4*2; i++ {
 		<-conn.outbound
 	}
+	roomsM.RLock()
 	if *rooms["#barenc"].key != "key1" {
 		t.Fatal("no room with key1")
 	}
 	if *rooms["#bazenc"].key != "key2" {
 		t.Fatal("no room with key2")
 	}
+	roomsM.RUnlock()
 	if r := <-logSink; (r.what != "joined") || (r.where != "#barenc") || (r.who != "nick2") || (r.meta != true) {
 		t.Fatal("invalid join log event #barenc", r)
 	}
@@ -222,9 +228,11 @@ func TestJoin(t *testing.T) {
 	if r := <-conn.outbound; r != ":nick2!foo2@someclient MODE #barenc -k\r\n" {
 		t.Fatal("remove #barenc key", r)
 	}
+	roomsM.RLock()
 	if *rooms["#barenc"].key != "" {
 		t.Fatal("removing key from #barenc")
 	}
+	roomsM.RUnlock()
 	if r := <-logSink; (r.what != "removed channel key") || (r.where != "#barenc") || (r.who != "nick2") || (r.meta != true) {
 		t.Fatal("removed channel key log", r)
 	}