remove registeredChannelsMutex

This moves channel registration to an eventual consistency model,
where the in-memory datastructures (Channel and ChannelManager)
are the exclusive source of truth, and updates to them get persisted
asynchronously to the DB.

irc/channel.go

@@ -6,6 +6,7 @@
 package irc
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"time"
@@ -14,7 +15,10 @@ import (
 
 	"github.com/goshuirc/irc-go/ircmsg"
 	"github.com/oragono/oragono/irc/caps"
-	"github.com/tidwall/buntdb"
+)
+
+var (
+	ChannelAlreadyRegistered = errors.New("Channel is already registered")
 )
 
 // Channel represents a channel that clients can join.
@@ -29,6 +33,8 @@ type Channel struct {
 	nameCasefolded    string
 	server            *Server
 	createdTime       time.Time
+	registeredFounder string
+	registeredTime    time.Time
 	stateMutex        sync.RWMutex
 	topic             string
 	topicSetBy        string
@@ -38,7 +44,7 @@ type Channel struct {
 
 // NewChannel creates a new channel from a `Server` and a `name`
 // string, which must be unique on the server.
-func NewChannel(s *Server, name string, addDefaultModes bool) *Channel {
+func NewChannel(s *Server, name string, addDefaultModes bool, regInfo *RegisteredChannel) *Channel {
 	casefoldedName, err := CasefoldChannel(name)
 	if err != nil {
 		s.logger.Error("internal", fmt.Sprintf("Bad channel name %s: %v", name, err))
@@ -46,7 +52,8 @@ func NewChannel(s *Server, name string, addDefaultModes bool) *Channel {
 	}
 
 	channel := &Channel{
-		flags: make(ModeSet),
+		createdTime: time.Now(), // may be overwritten by applyRegInfo
+		flags:       make(ModeSet),
 		lists: map[Mode]*UserMaskSet{
 			BanMask:    NewUserMaskSet(),
 			ExceptMask: NewUserMaskSet(),
@@ -64,9 +71,80 @@ func NewChannel(s *Server, name string, addDefaultModes bool) *Channel {
 		}
 	}
 
+	if regInfo != nil {
+		channel.applyRegInfo(regInfo)
+	}
+
 	return channel
 }
 
+// read in channel state that was persisted in the DB
+func (channel *Channel) applyRegInfo(chanReg *RegisteredChannel) {
+	channel.registeredFounder = chanReg.Founder
+	channel.registeredTime = chanReg.RegisteredAt
+	channel.topic = chanReg.Topic
+	channel.topicSetBy = chanReg.TopicSetBy
+	channel.topicSetTime = chanReg.TopicSetTime
+	channel.name = chanReg.Name
+	channel.createdTime = chanReg.RegisteredAt
+	for _, mask := range chanReg.Banlist {
+		channel.lists[BanMask].Add(mask)
+	}
+	for _, mask := range chanReg.Exceptlist {
+		channel.lists[ExceptMask].Add(mask)
+	}
+	for _, mask := range chanReg.Invitelist {
+		channel.lists[InviteMask].Add(mask)
+	}
+}
+
+// obtain a consistent snapshot of the channel state that can be persisted to the DB
+func (channel *Channel) ExportRegistration(includeLists bool) (info RegisteredChannel) {
+	channel.stateMutex.RLock()
+	defer channel.stateMutex.RUnlock()
+
+	info.Name = channel.name
+	info.Topic = channel.topic
+	info.TopicSetBy = channel.topicSetBy
+	info.TopicSetTime = channel.topicSetTime
+	info.Founder = channel.registeredFounder
+	info.RegisteredAt = channel.registeredTime
+
+	if includeLists {
+		for mask := range channel.lists[BanMask].masks {
+			info.Banlist = append(info.Banlist, mask)
+		}
+		for mask := range channel.lists[ExceptMask].masks {
+			info.Exceptlist = append(info.Exceptlist, mask)
+		}
+		for mask := range channel.lists[InviteMask].masks {
+			info.Invitelist = append(info.Invitelist, mask)
+		}
+	}
+
+	return
+}
+
+// SetRegistered registers the channel, returning an error if it was already registered.
+func (channel *Channel) SetRegistered(founder string) error {
+	channel.stateMutex.Lock()
+	defer channel.stateMutex.Unlock()
+
+	if channel.registeredFounder != "" {
+		return ChannelAlreadyRegistered
+	}
+	channel.registeredFounder = founder
+	channel.registeredTime = time.Now()
+	return nil
+}
+
+// IsRegistered returns whether the channel is registered.
+func (channel *Channel) IsRegistered() bool {
+	channel.stateMutex.RLock()
+	defer channel.stateMutex.RUnlock()
+	return channel.registeredFounder != ""
+}
+
 func (channel *Channel) regenerateMembersCache() {
 	// this is eventually consistent even without holding stateMutex.Lock()
 	// throughout the update; all updates to `members` while holding Lock()
@@ -340,59 +418,25 @@ func (channel *Channel) Join(client *Client, key string) {
 	client.addChannel(channel)
 
 	// give channel mode if necessary
-	var newChannel bool
+	newChannel := firstJoin && !channel.IsRegistered()
 	var givenMode *Mode
-	client.server.registeredChannelsMutex.Lock()
+	if client.AccountName() == channel.registeredFounder {
-	defer client.server.registeredChannelsMutex.Unlock()
+		givenMode = &ChannelFounder
-	client.server.store.Update(func(tx *buntdb.Tx) error {
+	} else if newChannel {
-		chanReg := client.server.loadChannelNoMutex(tx, channel.nameCasefolded)
+		givenMode = &ChannelOperator
-
+	}
-		if chanReg == nil {
+	if givenMode != nil {
-			if firstJoin {
+		channel.stateMutex.Lock()
-				channel.stateMutex.Lock()
+		channel.members[client][*givenMode] = true
-				channel.createdTime = time.Now()
+		channel.stateMutex.Unlock()
-				channel.members[client][ChannelOperator] = true
+	}
-				channel.stateMutex.Unlock()
-				givenMode = &ChannelOperator
-				newChannel = true
-			}
-		} else {
-			// we should only do this on registered channels
-			if client.account != nil && client.account.Name == chanReg.Founder {
-				channel.stateMutex.Lock()
-				channel.members[client][ChannelFounder] = true
-				channel.stateMutex.Unlock()
-				givenMode = &ChannelFounder
-			}
-			if firstJoin {
-				// apply other details if new channel
-				channel.stateMutex.Lock()
-				channel.topic = chanReg.Topic
-				channel.topicSetBy = chanReg.TopicSetBy
-				channel.topicSetTime = chanReg.TopicSetTime
-				channel.name = chanReg.Name
-				channel.createdTime = chanReg.RegisteredAt
-				for _, mask := range chanReg.Banlist {
-					channel.lists[BanMask].Add(mask)
-				}
-				for _, mask := range chanReg.Exceptlist {
-					channel.lists[ExceptMask].Add(mask)
-				}
-				for _, mask := range chanReg.Invitelist {
-					channel.lists[InviteMask].Add(mask)
-				}
-				channel.stateMutex.Unlock()
-			}
-		}
-		return nil
-	})
 
 	if client.capabilities.Has(caps.ExtendedJoin) {
 		client.Send(nil, client.nickMaskString, "JOIN", channel.name, client.account.Name, client.realname)
 	} else {
 		client.Send(nil, client.nickMaskString, "JOIN", channel.name)
 	}
-	// don't sent topic when it's an entirely new channel
+	// don't send topic when it's an entirely new channel
 	if !newChannel {
 		channel.SendTopic(client)
 	}
@@ -468,23 +512,7 @@ func (channel *Channel) SetTopic(client *Client, topic string) {
 		member.Send(nil, client.nickMaskString, "TOPIC", channel.name, topic)
 	}
 
-	// update saved channel topic for registered chans
+	go channel.server.channelRegistry.StoreChannel(channel, false)
-	client.server.registeredChannelsMutex.Lock()
-	defer client.server.registeredChannelsMutex.Unlock()
-
-	client.server.store.Update(func(tx *buntdb.Tx) error {
-		chanInfo := client.server.loadChannelNoMutex(tx, channel.nameCasefolded)
-
-		if chanInfo == nil {
-			return nil
-		}
-
-		chanInfo.Topic = topic
-		chanInfo.TopicSetBy = client.nickMaskString
-		chanInfo.TopicSetTime = time.Now()
-		client.server.saveChannelNoMutex(tx, channel.nameCasefolded, *chanInfo)
-		return nil
-	})
 }
 
 // CanSpeak returns true if the client can speak on this channel.

irc/channelmanager.go

@@ -59,11 +59,21 @@ func (cm *ChannelManager) Join(client *Client, name string, key string) error {
 	cm.Lock()
 	entry := cm.chans[casefoldedName]
 	if entry == nil {
-		entry = &channelManagerEntry{
+		// XXX give up the lock to check for a registration, then check again
-			channel:      NewChannel(server, name, true),
+		// to see if we need to create the channel. we could solve this by doing LoadChannel
-			pendingJoins: 0,
+		// outside the lock initially on every join, so this is best thought of as an
+		// optimization to avoid that.
+		cm.Unlock()
+		info := client.server.channelRegistry.LoadChannel(casefoldedName)
+		cm.Lock()
+		entry = cm.chans[casefoldedName]
+		if entry == nil {
+			entry = &channelManagerEntry{
+				channel:      NewChannel(server, name, true, info),
+				pendingJoins: 0,
+			}
+			cm.chans[casefoldedName] = entry
 		}
-		cm.chans[casefoldedName] = entry
 	}
 	entry.pendingJoins += 1
 	cm.Unlock()
@@ -85,7 +95,12 @@ func (cm *ChannelManager) maybeCleanup(entry *channelManagerEntry, afterJoin boo
 	if afterJoin {
 		entry.pendingJoins -= 1
 	}
-	if entry.channel.IsEmpty() && entry.pendingJoins == 0 {
+	// TODO(slingamn) right now, registered channels cannot be cleaned up.
+	// this is because once ChannelManager becomes the source of truth about a channel,
+	// we can't move the source of truth back to the database unless we do an ACID
+	// store while holding the ChannelManager's Lock(). This is pending more decisions
+	// about where the database transaction lock fits into the overall lock model.
+	if !entry.channel.IsRegistered() && entry.channel.IsEmpty() && entry.pendingJoins == 0 {
 		// reread the name, handling the case where the channel was renamed
 		casefoldedName := entry.channel.NameCasefolded()
 		delete(cm.chans, casefoldedName)

irc/channelreg.go

@@ -4,9 +4,9 @@
 package irc
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
+	"sync"
 	"time"
 
 	"encoding/json"
@@ -14,6 +14,9 @@ import (
 	"github.com/tidwall/buntdb"
 )
 
+// this is exclusively the *persistence* layer for channel registration;
+// channel creation/tracking/destruction is in channelmanager.go
+
 const (
 	keyChannelExists       = "channel.exists %s"
 	keyChannelName         = "channel.name %s" // stores the 'preferred name' of the channel, not casemapped
@@ -28,7 +31,18 @@ const (
 )
 
 var (
-	errChanExists = errors.New("Channel already exists")
+	channelKeyStrings = []string{
+		keyChannelExists,
+		keyChannelName,
+		keyChannelRegTime,
+		keyChannelFounder,
+		keyChannelTopic,
+		keyChannelTopicSetBy,
+		keyChannelTopicSetTime,
+		keyChannelBanlist,
+		keyChannelExceptlist,
+		keyChannelInvitelist,
+	}
 )
 
 // RegisteredChannel holds details about a given registered channel.
@@ -53,62 +67,142 @@ type RegisteredChannel struct {
 	Invitelist []string
 }
 
-// deleteChannelNoMutex deletes a given channel from our store.
+type ChannelRegistry struct {
-func (server *Server) deleteChannelNoMutex(tx *buntdb.Tx, channelKey string) {
+	// this serializes operations of the form (read channel state, synchronously persist it);
-	tx.Delete(fmt.Sprintf(keyChannelExists, channelKey))
+	// this is enough to guarantee eventual consistency of the database with the
-	server.registeredChannels[channelKey] = nil
+	// ChannelManager and Channel objects, which are the source of truth.
+	// Wwe could use the buntdb RW transaction lock for this purpose but we share
+	// that with all the other modules, so let's not.
+	sync.Mutex // tier 2
+	server     *Server
+	channels   map[string]*RegisteredChannel
 }
 
-// loadChannelNoMutex loads a channel from the store.
+func NewChannelRegistry(server *Server) *ChannelRegistry {
-func (server *Server) loadChannelNoMutex(tx *buntdb.Tx, channelKey string) *RegisteredChannel {
+	return &ChannelRegistry{
-	// return loaded chan if it already exists
+		server: server,
-	if server.registeredChannels[channelKey] != nil {
-		return server.registeredChannels[channelKey]
 	}
-	_, err := tx.Get(fmt.Sprintf(keyChannelExists, channelKey))
+}
-	if err == buntdb.ErrNotFound {
+
-		// chan does not already exist, return
+// StoreChannel obtains a consistent view of a channel, then persists it to the store.
+func (reg *ChannelRegistry) StoreChannel(channel *Channel, includeLists bool) {
+	if !reg.server.ChannelRegistrationEnabled() {
+		return
+	}
+
+	reg.Lock()
+	defer reg.Unlock()
+
+	key := channel.NameCasefolded()
+	info := channel.ExportRegistration(includeLists)
+	if info.Founder == "" {
+		// sanity check, don't try to store an unregistered channel
+		return
+	}
+
+	reg.server.store.Update(func(tx *buntdb.Tx) error {
+		reg.saveChannel(tx, key, info, includeLists)
+		return nil
+	})
+}
+
+// LoadChannel loads a channel from the store.
+func (reg *ChannelRegistry) LoadChannel(nameCasefolded string) (info *RegisteredChannel) {
+	if !reg.server.ChannelRegistrationEnabled() {
 		return nil
 	}
 
-	// channel exists, load it
+	channelKey := nameCasefolded
-	name, _ := tx.Get(fmt.Sprintf(keyChannelName, channelKey))
+	// nice to have: do all JSON (de)serialization outside of the buntdb transaction
-	regTime, _ := tx.Get(fmt.Sprintf(keyChannelRegTime, channelKey))
+	reg.server.store.View(func(tx *buntdb.Tx) error {
-	regTimeInt, _ := strconv.ParseInt(regTime, 10, 64)
+		_, err := tx.Get(fmt.Sprintf(keyChannelExists, channelKey))
-	founder, _ := tx.Get(fmt.Sprintf(keyChannelFounder, channelKey))
+		if err == buntdb.ErrNotFound {
-	topic, _ := tx.Get(fmt.Sprintf(keyChannelTopic, channelKey))
+			// chan does not already exist, return
-	topicSetBy, _ := tx.Get(fmt.Sprintf(keyChannelTopicSetBy, channelKey))
+			return nil
-	topicSetTime, _ := tx.Get(fmt.Sprintf(keyChannelTopicSetTime, channelKey))
+		}
-	topicSetTimeInt, _ := strconv.ParseInt(topicSetTime, 10, 64)
-	banlistString, _ := tx.Get(fmt.Sprintf(keyChannelBanlist, channelKey))
-	exceptlistString, _ := tx.Get(fmt.Sprintf(keyChannelExceptlist, channelKey))
-	invitelistString, _ := tx.Get(fmt.Sprintf(keyChannelInvitelist, channelKey))
 
-	var banlist []string
+		// channel exists, load it
-	_ = json.Unmarshal([]byte(banlistString), &banlist)
+		name, _ := tx.Get(fmt.Sprintf(keyChannelName, channelKey))
-	var exceptlist []string
+		regTime, _ := tx.Get(fmt.Sprintf(keyChannelRegTime, channelKey))
-	_ = json.Unmarshal([]byte(exceptlistString), &exceptlist)
+		regTimeInt, _ := strconv.ParseInt(regTime, 10, 64)
-	var invitelist []string
+		founder, _ := tx.Get(fmt.Sprintf(keyChannelFounder, channelKey))
-	_ = json.Unmarshal([]byte(invitelistString), &invitelist)
+		topic, _ := tx.Get(fmt.Sprintf(keyChannelTopic, channelKey))
+		topicSetBy, _ := tx.Get(fmt.Sprintf(keyChannelTopicSetBy, channelKey))
+		topicSetTime, _ := tx.Get(fmt.Sprintf(keyChannelTopicSetTime, channelKey))
+		topicSetTimeInt, _ := strconv.ParseInt(topicSetTime, 10, 64)
+		banlistString, _ := tx.Get(fmt.Sprintf(keyChannelBanlist, channelKey))
+		exceptlistString, _ := tx.Get(fmt.Sprintf(keyChannelExceptlist, channelKey))
+		invitelistString, _ := tx.Get(fmt.Sprintf(keyChannelInvitelist, channelKey))
 
-	chanInfo := RegisteredChannel{
+		var banlist []string
-		Name:         name,
+		_ = json.Unmarshal([]byte(banlistString), &banlist)
-		RegisteredAt: time.Unix(regTimeInt, 0),
+		var exceptlist []string
-		Founder:      founder,
+		_ = json.Unmarshal([]byte(exceptlistString), &exceptlist)
-		Topic:        topic,
+		var invitelist []string
-		TopicSetBy:   topicSetBy,
+		_ = json.Unmarshal([]byte(invitelistString), &invitelist)
-		TopicSetTime: time.Unix(topicSetTimeInt, 0),
-		Banlist:      banlist,
-		Exceptlist:   exceptlist,
-		Invitelist:   invitelist,
-	}
-	server.registeredChannels[channelKey] = &chanInfo
 
-	return &chanInfo
+		info = &RegisteredChannel{
+			Name:         name,
+			RegisteredAt: time.Unix(regTimeInt, 0),
+			Founder:      founder,
+			Topic:        topic,
+			TopicSetBy:   topicSetBy,
+			TopicSetTime: time.Unix(topicSetTimeInt, 0),
+			Banlist:      banlist,
+			Exceptlist:   exceptlist,
+			Invitelist:   invitelist,
+		}
+		return nil
+	})
+
+	return info
 }
 
-// saveChannelNoMutex saves a channel to the store.
+// Rename handles the persistence part of a channel rename: the channel is
-func (server *Server) saveChannelNoMutex(tx *buntdb.Tx, channelKey string, channelInfo RegisteredChannel) {
+// persisted under its new name, and the old name is cleaned up if necessary.
+func (reg *ChannelRegistry) Rename(channel *Channel, casefoldedOldName string) {
+	if !reg.server.ChannelRegistrationEnabled() {
+		return
+	}
+
+	reg.Lock()
+	defer reg.Unlock()
+
+	includeLists := true
+	oldKey := casefoldedOldName
+	key := channel.NameCasefolded()
+	info := channel.ExportRegistration(includeLists)
+	if info.Founder == "" {
+		return
+	}
+
+	reg.server.store.Update(func(tx *buntdb.Tx) error {
+		reg.deleteChannel(tx, oldKey, info)
+		reg.saveChannel(tx, key, info, includeLists)
+		return nil
+	})
+}
+
+// delete a channel, unless it was overwritten by another registration of the same channel
+func (reg *ChannelRegistry) deleteChannel(tx *buntdb.Tx, key string, info RegisteredChannel) {
+	_, err := tx.Get(fmt.Sprintf(keyChannelExists, key))
+	if err == nil {
+		regTime, _ := tx.Get(fmt.Sprintf(keyChannelRegTime, key))
+		regTimeInt, _ := strconv.ParseInt(regTime, 10, 64)
+		registeredAt := time.Unix(regTimeInt, 0)
+		founder, _ := tx.Get(fmt.Sprintf(keyChannelFounder, key))
+
+		// to see if we're deleting the right channel, confirm the founder and the registration time
+		if founder == info.Founder && registeredAt == info.RegisteredAt {
+			for _, keyFmt := range channelKeyStrings {
+				tx.Delete(fmt.Sprintf(keyFmt, key))
+			}
+		}
+	}
+}
+
+// saveChannel saves a channel to the store.
+func (reg *ChannelRegistry) saveChannel(tx *buntdb.Tx, channelKey string, channelInfo RegisteredChannel, includeLists bool) {
 	tx.Set(fmt.Sprintf(keyChannelExists, channelKey), "1", nil)
 	tx.Set(fmt.Sprintf(keyChannelName, channelKey), channelInfo.Name, nil)
 	tx.Set(fmt.Sprintf(keyChannelRegTime, channelKey), strconv.FormatInt(channelInfo.RegisteredAt.Unix(), 10), nil)
@@ -117,12 +211,12 @@ func (server *Server) saveChannelNoMutex(tx *buntdb.Tx, channelKey string, chann
 	tx.Set(fmt.Sprintf(keyChannelTopicSetBy, channelKey), channelInfo.TopicSetBy, nil)
 	tx.Set(fmt.Sprintf(keyChannelTopicSetTime, channelKey), strconv.FormatInt(channelInfo.TopicSetTime.Unix(), 10), nil)
 
-	banlistString, _ := json.Marshal(channelInfo.Banlist)
+	if includeLists {
-	tx.Set(fmt.Sprintf(keyChannelBanlist, channelKey), string(banlistString), nil)
+		banlistString, _ := json.Marshal(channelInfo.Banlist)
-	exceptlistString, _ := json.Marshal(channelInfo.Exceptlist)
+		tx.Set(fmt.Sprintf(keyChannelBanlist, channelKey), string(banlistString), nil)
-	tx.Set(fmt.Sprintf(keyChannelExceptlist, channelKey), string(exceptlistString), nil)
+		exceptlistString, _ := json.Marshal(channelInfo.Exceptlist)
-	invitelistString, _ := json.Marshal(channelInfo.Invitelist)
+		tx.Set(fmt.Sprintf(keyChannelExceptlist, channelKey), string(exceptlistString), nil)
-	tx.Set(fmt.Sprintf(keyChannelInvitelist, channelKey), string(invitelistString), nil)
+		invitelistString, _ := json.Marshal(channelInfo.Invitelist)
-
+		tx.Set(fmt.Sprintf(keyChannelInvitelist, channelKey), string(invitelistString), nil)
-	server.registeredChannels[channelKey] = &channelInfo
+	}
 }

irc/chanserv.go

@@ -6,12 +6,10 @@ package irc
 import (
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/goshuirc/irc-go/ircfmt"
 	"github.com/goshuirc/irc-go/ircmsg"
 	"github.com/oragono/oragono/irc/sno"
-	"github.com/tidwall/buntdb"
 )
 
 // csHandler handles the /CS and /CHANSERV commands
@@ -56,9 +54,6 @@ func (server *Server) chanservReceivePrivmsg(client *Client, message string) {
 			return
 		}
 
-		server.registeredChannelsMutex.Lock()
-		defer server.registeredChannelsMutex.Unlock()
-
 		channelName := params[1]
 		channelKey, err := CasefoldChannel(channelName)
 		if err != nil {
@@ -67,57 +62,41 @@ func (server *Server) chanservReceivePrivmsg(client *Client, message string) {
 		}
 
 		channelInfo := server.channels.Get(channelKey)
-		if channelInfo == nil {
+		if channelInfo == nil || !channelInfo.ClientIsAtLeast(client, ChannelOperator) {
 			client.ChanServNotice("You must be an oper on the channel to register it")
 			return
 		}
 
-		if !channelInfo.ClientIsAtLeast(client, ChannelOperator) {
+		if client.account == &NoAccount {
-			client.ChanServNotice("You must be an oper on the channel to register it")
+			client.ChanServNotice("You must be logged in to register a channel")
 			return
 		}
 
-		server.store.Update(func(tx *buntdb.Tx) error {
+		// this provides the synchronization that allows exactly one registration of the channel:
-			currentChan := server.loadChannelNoMutex(tx, channelKey)
+		err = channelInfo.SetRegistered(client.AccountName())
-			if currentChan != nil {
+		if err != nil {
-				client.ChanServNotice("Channel is already registered")
+			client.ChanServNotice(err.Error())
-				return nil
+			return
+		}
+
+		// registration was successful: make the database reflect it
+		go server.channelRegistry.StoreChannel(channelInfo, true)
+
+		client.ChanServNotice(fmt.Sprintf("Channel %s successfully registered", channelName))
+
+		server.logger.Info("chanserv", fmt.Sprintf("Client %s registered channel %s", client.nick, channelName))
+		server.snomasks.Send(sno.LocalChannels, fmt.Sprintf(ircfmt.Unescape("Channel registered $c[grey][$r%s$c[grey]] by $c[grey][$r%s$c[grey]]"), channelName, client.nickMaskString))
+
+		// give them founder privs
+		change := channelInfo.applyModeMemberNoMutex(client, ChannelFounder, Add, client.NickCasefolded())
+		if change != nil {
+			//TODO(dan): we should change the name of String and make it return a slice here
+			//TODO(dan): unify this code with code in modes.go
+			args := append([]string{channelName}, strings.Split(change.String(), " ")...)
+			for _, member := range channelInfo.Members() {
+				member.Send(nil, fmt.Sprintf("ChanServ!services@%s", client.server.name), "MODE", args...)
 			}
-
+		}
-			account := client.account
-			if account == &NoAccount {
-				client.ChanServNotice("You must be logged in to register a channel")
-				return nil
-			}
-
-			chanRegInfo := RegisteredChannel{
-				Name:         channelName,
-				RegisteredAt: time.Now(),
-				Founder:      account.Name,
-				Topic:        channelInfo.topic,
-				TopicSetBy:   channelInfo.topicSetBy,
-				TopicSetTime: channelInfo.topicSetTime,
-			}
-			server.saveChannelNoMutex(tx, channelKey, chanRegInfo)
-
-			client.ChanServNotice(fmt.Sprintf("Channel %s successfully registered", channelName))
-
-			server.logger.Info("chanserv", fmt.Sprintf("Client %s registered channel %s", client.nick, channelName))
-			server.snomasks.Send(sno.LocalChannels, fmt.Sprintf(ircfmt.Unescape("Channel registered $c[grey][$r%s$c[grey]] by $c[grey][$r%s$c[grey]]"), channelName, client.nickMaskString))
-
-			// give them founder privs
-			change := channelInfo.applyModeMemberNoMutex(client, ChannelFounder, Add, client.nickCasefolded)
-			if change != nil {
-				//TODO(dan): we should change the name of String and make it return a slice here
-				//TODO(dan): unify this code with code in modes.go
-				args := append([]string{channelName}, strings.Split(change.String(), " ")...)
-				for _, member := range channelInfo.Members() {
-					member.Send(nil, fmt.Sprintf("ChanServ!services@%s", client.server.name), "MODE", args...)
-				}
-			}
-
-			return nil
-		})
 	} else {
 		client.ChanServNotice("Sorry, I don't know that command")
 	}

irc/getters.go

@@ -47,6 +47,12 @@ func (server *Server) DefaultChannelModes() Modes {
 	return server.defaultChannelModes
 }
 
+func (server *Server) ChannelRegistrationEnabled() bool {
+	server.configurableStateMutex.RLock()
+	defer server.configurableStateMutex.RUnlock()
+	return server.channelRegistrationEnabled
+}
+
 func (client *Client) Nick() string {
 	client.stateMutex.RLock()
 	defer client.stateMutex.RUnlock()
@@ -95,6 +101,12 @@ func (client *Client) Destroyed() bool {
 	return client.isDestroyed
 }
 
+func (client *Client) AccountName() string {
+	client.stateMutex.RLock()
+	defer client.stateMutex.RUnlock()
+	return client.account.Name
+}
+
 func (client *Client) HasMode(mode Mode) bool {
 	client.stateMutex.RLock()
 	defer client.stateMutex.RUnlock()
@@ -174,6 +186,12 @@ func (channel *Channel) HasMode(mode Mode) bool {
 	return channel.flags[mode]
 }
 
+func (channel *Channel) Founder() string {
+	channel.stateMutex.RLock()
+	defer channel.stateMutex.RUnlock()
+	return channel.registeredFounder
+}
+
 // set a channel mode, return whether it was already set
 func (channel *Channel) setMode(mode Mode, enable bool) (already bool) {
 	channel.stateMutex.Lock()

irc/modes.go

@@ -11,7 +11,6 @@ import (
 
 	"github.com/goshuirc/irc-go/ircmsg"
 	"github.com/oragono/oragono/irc/sno"
-	"github.com/tidwall/buntdb"
 )
 
 // ModeOp is an operation performed with modes
@@ -645,39 +644,9 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 		}
 	}
 
-	server.registeredChannelsMutex.Lock()
+	if (banlistUpdated || exceptlistUpdated || invexlistUpdated) && channel.IsRegistered() {
-	if 0 < len(applied) && server.registeredChannels[channel.nameCasefolded] != nil && (banlistUpdated || exceptlistUpdated || invexlistUpdated) {
+		go server.channelRegistry.StoreChannel(channel, true)
-		server.store.Update(func(tx *buntdb.Tx) error {
-			chanInfo := server.loadChannelNoMutex(tx, channel.nameCasefolded)
-
-			if banlistUpdated {
-				var banlist []string
-				for mask := range channel.lists[BanMask].masks {
-					banlist = append(banlist, mask)
-				}
-				chanInfo.Banlist = banlist
-			}
-			if exceptlistUpdated {
-				var exceptlist []string
-				for mask := range channel.lists[ExceptMask].masks {
-					exceptlist = append(exceptlist, mask)
-				}
-				chanInfo.Exceptlist = exceptlist
-			}
-			if invexlistUpdated {
-				var invitelist []string
-				for mask := range channel.lists[InviteMask].masks {
-					invitelist = append(invitelist, mask)
-				}
-				chanInfo.Invitelist = invitelist
-			}
-
-			server.saveChannelNoMutex(tx, channel.nameCasefolded, *chanInfo)
-
-			return nil
-		})
 	}
-	server.registeredChannelsMutex.Unlock()
 
 	// send out changes
 	if len(applied) > 0 {

irc/server.go

@@ -84,6 +84,7 @@ type Server struct {
 	accounts                     map[string]*ClientAccount
 	channelRegistrationEnabled   bool
 	channels                     *ChannelManager
+	channelRegistry              *ChannelRegistry
 	checkIdent                   bool
 	clients                      *ClientLookupSet
 	commands                     chan Command
@@ -112,8 +113,6 @@ type Server struct {
 	password                     []byte
 	passwords                    *passwd.SaltedManager
 	recoverFromErrors            bool
-	registeredChannels           map[string]*RegisteredChannel
-	registeredChannelsMutex      sync.RWMutex
 	rehashMutex                  sync.Mutex
 	rehashSignal                 chan os.Signal
 	proxyAllowedFrom             []string
@@ -158,7 +157,6 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) {
 		logger:              logger,
 		monitorManager:      NewMonitorManager(),
 		newConns:            make(chan clientConn),
-		registeredChannels:  make(map[string]*RegisteredChannel),
 		rehashSignal:        make(chan os.Signal, 1),
 		signals:             make(chan os.Signal, len(ServerExitSignals)),
 		snomasks:            NewSnoManager(),
@@ -558,10 +556,6 @@ func pongHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 func renameHandler(server *Server, client *Client, msg ircmsg.IrcMessage) (result bool) {
 	result = false
 
-	// TODO(slingamn, #152) clean up locking here
-	server.registeredChannelsMutex.Lock()
-	defer server.registeredChannelsMutex.Unlock()
-
 	errorResponse := func(err error, name string) {
 		// TODO: send correct error codes, e.g., ERR_CANNOTRENAME, ERR_CHANNAMEINUSE
 		var code string
@@ -591,11 +585,6 @@ func renameHandler(server *Server, client *Client, msg ircmsg.IrcMessage) (resul
 		errorResponse(InvalidChannelName, oldName)
 		return
 	}
-	casefoldedNewName, err := CasefoldChannel(newName)
-	if err != nil {
-		errorResponse(InvalidChannelName, newName)
-		return
-	}
 
 	reason := "No reason"
 	if 2 < len(msg.Params) {
@@ -613,20 +602,8 @@ func renameHandler(server *Server, client *Client, msg ircmsg.IrcMessage) (resul
 		return
 	}
 
-	var canEdit bool
+	founder := channel.Founder()
-	server.store.Update(func(tx *buntdb.Tx) error {
+	if founder != "" && founder != client.AccountName() {
-		chanReg := server.loadChannelNoMutex(tx, casefoldedOldName)
-		if chanReg == nil || !client.LoggedIntoAccount() || client.account.Name == chanReg.Founder {
-			canEdit = true
-		}
-
-		chanReg = server.loadChannelNoMutex(tx, casefoldedNewName)
-		if chanReg != nil {
-			canEdit = false
-		}
-		return nil
-	})
-	if !canEdit {
 		//TODO(dan): Change this to ERR_CANNOTRENAME
 		client.Send(nil, server.name, ERR_UNKNOWNERROR, client.nick, "RENAME", oldName, "Only channel founders can change registered channels")
 		return false
@@ -639,20 +616,8 @@ func renameHandler(server *Server, client *Client, msg ircmsg.IrcMessage) (resul
 		return
 	}
 
-	// rename stored channel info if any exists
+	// rename succeeded, persist it
-	server.store.Update(func(tx *buntdb.Tx) error {
+	go server.channelRegistry.Rename(channel, casefoldedOldName)
-		chanReg := server.loadChannelNoMutex(tx, casefoldedOldName)
-		if chanReg == nil {
-			return nil
-		}
-
-		server.deleteChannelNoMutex(tx, casefoldedOldName)
-
-		chanReg.Name = newName
-
-		server.saveChannelNoMutex(tx, casefoldedNewName, *chanReg)
-		return nil
-	})
 
 	// send RENAME messages
 	for _, mcl := range channel.Members() {
@@ -1494,6 +1459,9 @@ func (server *Server) loadDatastore(datastorePath string) error {
 	if err != nil {
 		return fmt.Errorf("Could not load salt: %s", err.Error())
 	}
+
+	server.channelRegistry = NewChannelRegistry(server)
+
 	return nil
 }