]> Cypherpunks.ru repositories - goircd.git/commitdiff
Various race preventing locks
authorSergey Matveev <stargrave@stargrave.org>
Sat, 26 Mar 2016 14:10:04 +0000 (17:10 +0300)
committerSergey Matveev <stargrave@stargrave.org>
Sat, 26 Mar 2016 14:10:04 +0000 (17:10 +0300)
client.go
daemon.go
room.go
room_test.go

index e90d51220fee0eca4ab9d3d64ae0eef07a730ccb..640a58a5cae90a356944af8a1e2bc64e33340c96 100644 (file)
--- a/client.go
+++ b/client.go
@@ -51,7 +51,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
@@ -62,7 +62,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()
 }
 
index b5c3e56f3e97ad6f933436f55b6ad89eb68f3e3c..d46ee07fe0944ea74e63694bd6787cbb253aeb6e 100644 (file)
--- a/daemon.go
+++ b/daemon.go
@@ -40,18 +40,23 @@ const (
 var (
        RENickname = regexp.MustCompile("^[a-zA-Z0-9-]{1,24}$")
 
+       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
-
-       clients map[*Client]struct{} = make(map[*Client]struct{})
+       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 {
diff --git a/room.go b/room.go
index dcad9d4be34210c68d8a559eff346b35fcdca63f..d5693e851bdea6406c9c33c107f75dce22c59d99 100644 (file)
--- a/room.go
+++ b/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 {
-       return *room.name
+func (room *Room) String() (name string) {
+       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))
-                       logSink <- LogEvent{*room.name, *client.nickname, "joined", true}
+                       room.Broadcast(fmt.Sprintf(":%s JOIN %s", client, room.String()))
+                       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("366", *room.name, "End of NAMES list")
+                       client.ReplyNicknamed("353", "=", room.String(), strings.Join(nicknames, " "))
+                       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,
index 58f4158b9a7b22ce896da23b38922001c72ae2fe..8618fb9a3228509666a86d3058a65e427e224793 100644 (file)
@@ -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)
        }