GitHub/ergo
3
0
Fork 0
mirror of https://github.com/ergochat/ergo.git synced 2024-11-22 03:49:27 +01:00
Code Issues Projects Releases Wiki Activity

refactor ClientManager

Browse Source
This commit is contained in:
Shivaram Lingamneni 2017-11-22 04:41:11 -05:00
parent 2cbbec567c
commit 52b0fb71e7
13 changed files with 146 additions and 206 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
DEVELOPING.md
View File

@ -80,3 +80,22 @@ To debug a hang, the best thing to do is to get a stack trace. Go's nice, and yo
$ kill -ABRT <procid>
This will kill Oragono and print out a stack trace for you to take a look at.
## Concurrency design
Oragono involves a fair amount of shared state. Here are some of the main points:
1. Each client has a separate goroutine that listens for incoming messages and synchronously processes them.
1. All sends to clients are asynchronous; `client.Send` appends the message to a queue, which is then processed on a separate goroutine. It is always safe to call `client.Send`.
1. The server has a few of its own goroutines, for listening on sockets and handing off new client connections to their dedicated goroutines.
1. A few tasks are done asynchronously in ad-hoc goroutines.
In consequence, there is a lot of state (in particular, server and channel state) that can be read and written from multiple goroutines. This state is protected with mutexes. To avoid deadlocks, mutexes are arranged in "tiers"; while holding a mutex of one tier, you're only allowed to acquire mutexes of a strictly *higher* tier. The tiers are:
1. Tier 1 mutexes: these are the "innermost" mutexes. They typically protect getters and setters on objects, or invariants that are local to the state of a single object. Example: `Channel.stateMutex`.
1. Tier 2 mutexes: these protect some invariants of their own, but also need to access fields on other objects that themselves require synchronization. Example: `ChannelManager.RWMutex`.
1. Tier 3 mutexes: these protect macroscopic operations, where it doesn't make sense for more than one to occur concurrently. Example; `Server.rehashMutex`, which prevents rehashes from overlapping.
There are some mutexes that are "tier 0": anything in a subpackage of `irc` (e.g., `irc/logger` or `irc/connection_limits`) shouldn't acquire mutexes defined in `irc`.
We are using `buntdb` for persistence; a `buntdb.DB` has an `RWMutex` inside it, with read-write transactions getting the `Lock()` and read-only transactions getting the `RLock()`. We haven't completely decided where this lock fits into the overall lock model. For now, it's probably better to err on the side of caution: if possible, don't acquire new locks inside the `buntdb` transaction, and be careful about what locks are held around the transaction as well.

2
README.md
View File

@ -68,7 +68,7 @@ The `stable` branch contains the latest release. You can run this for a producti
[![Build Status](https://travis-ci.org/oragono/oragono.svg?branch=master)](https://travis-ci.org/oragono/oragono)
Clone the appropriate branch. From the root folder, run `make` to generate all release files for all of our target OSes:
Clone the appropriate branch. If necessary, do `git submodule update --init` to set up vendored dependencies. From the root folder, run `make` to generate all release files for all of our target OSes:
```
make
```

2
irc/channel.go
View File

@ -35,7 +35,7 @@ type Channel struct {
createdTime time.Time
registeredFounder string
registeredTime time.Time
stateMutex sync.RWMutex
stateMutex sync.RWMutex // tier 1
topic string
topicSetBy string
topicSetTime time.Time

39
irc/client.go
View File

@ -73,7 +73,7 @@ type Client struct {
saslValue string
server *Server
socket *Socket
stateMutex sync.RWMutex // generic protection for mutable state
stateMutex sync.RWMutex // tier 1
username string
vhost string
whoisLine string
@ -313,11 +313,15 @@ func (client *Client) IdleSeconds() uint64 {
// HasNick returns true if the client's nickname is set (used in registration).
func (client *Client) HasNick() bool {
client.stateMutex.RLock()
defer client.stateMutex.RUnlock()
return client.nick != "" && client.nick != "*"
}
// HasUsername returns true if the client's username is set (used in registration).
func (client *Client) HasUsername() bool {
client.stateMutex.RLock()
defer client.stateMutex.RUnlock()
return client.username != "" && client.username != "*"
}
@ -403,6 +407,7 @@ func (client *Client) updateNickMask(nick string) {
}
client.stateMutex.Lock()
defer client.stateMutex.Unlock()
if len(client.vhost) > 0 {
client.hostname = client.vhost
@ -419,8 +424,6 @@ func (client *Client) updateNickMask(nick string) {
client.nickMaskString = nickMaskString
client.nickMaskCasefolded = nickMaskCasefolded
client.stateMutex.Unlock()
}
// AllNickmasks returns all the possible nickmasks for the client.
@ -449,36 +452,6 @@ func (client *Client) AllNickmasks() []string {
return masks
}
// SetNickname sets the very first nickname for the client.
func (client *Client) SetNickname(nickname string) error {
if client.HasNick() {
client.server.logger.Error("nick", fmt.Sprintf("%s nickname already set, something is wrong with server consistency", client.nickMaskString))
return ErrNickAlreadySet
}
err := client.server.clients.Add(client, nickname)
if err == nil {
client.updateNick(nickname)
}
return err
}
// ChangeNickname changes the existing nickname of the client.
func (client *Client) ChangeNickname(nickname string) error {
origNickMask := client.nickMaskString
err := client.server.clients.Replace(client.nick, nickname, client)
if err == nil {
client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s", client.nick, nickname))
client.server.snomasks.Send(sno.LocalNicks, fmt.Sprintf(ircfmt.Unescape("$%s$r changed nickname to %s"), client.nick, nickname))
client.server.whoWas.Append(client)
client.updateNickMask(nickname)
for friend := range client.Friends() {
friend.Send(nil, origNickMask, "NICK", nickname)
}
}
return err
}
// LoggedIntoAccount returns true if this client is logged into an account.
func (client *Client) LoggedIntoAccount() bool {
return client.account != nil && client.account != &NoAccount

163
irc/client_lookup_set.go
View File

@ -20,7 +20,6 @@ import (
var (
ErrNickMissing = errors.New("nick missing")
ErrNicknameInUse = errors.New("nickname in use")
ErrNicknameMismatch = errors.New("nickname mismatch")
)
// ExpandUserHost takes a userhost, and returns an expanded version.
@ -37,132 +36,108 @@ func ExpandUserHost(userhost string) (expanded string) {
return
}
// ClientLookupSet represents a way to store, search and lookup clients.
type ClientLookupSet struct {
ByNickMutex sync.RWMutex
ByNick map[string]*Client
// ClientManager keeps track of clients by nick, enforcing uniqueness of casefolded nicks
type ClientManager struct {
sync.RWMutex // tier 2
byNick map[string]*Client
}
// NewClientLookupSet returns a new lookup set.
func NewClientLookupSet() *ClientLookupSet {
return &ClientLookupSet{
ByNick: make(map[string]*Client),
// NewClientManager returns a new ClientManager.
func NewClientManager() *ClientManager {
return &ClientManager{
byNick: make(map[string]*Client),
}
}
// Count returns how many clients are in the lookup set.
func (clients *ClientLookupSet) Count() int {
clients.ByNickMutex.RLock()
defer clients.ByNickMutex.RUnlock()
count := len(clients.ByNick)
// Count returns how many clients are in the manager.
func (clients *ClientManager) Count() int {
clients.RLock()
defer clients.RUnlock()
count := len(clients.byNick)
return count
}
// Has returns whether or not the given client exists.
//TODO(dan): This seems like ripe ground for a race, if code does Has then Get, and assumes the Get will return a client.
func (clients *ClientLookupSet) Has(nick string) bool {
// Get retrieves a client from the manager, if they exist.
func (clients *ClientManager) Get(nick string) *Client {
casefoldedName, err := CasefoldName(nick)
if err == nil {
return false
}
clients.ByNickMutex.RLock()
defer clients.ByNickMutex.RUnlock()
_, exists := clients.ByNick[casefoldedName]
return exists
}
// getNoMutex is used internally, for getting clients when no mutex is required (i.e. is already set).
func (clients *ClientLookupSet) getNoMutex(nick string) *Client {
casefoldedName, err := CasefoldName(nick)
if err == nil {
cli := clients.ByNick[casefoldedName]
clients.RLock()
defer clients.RUnlock()
cli := clients.byNick[casefoldedName]
return cli
}
return nil
}
// Get retrieves a client from the set, if they exist.
func (clients *ClientLookupSet) Get(nick string) *Client {
casefoldedName, err := CasefoldName(nick)
if err == nil {
clients.ByNickMutex.RLock()
defer clients.ByNickMutex.RUnlock()
cli := clients.ByNick[casefoldedName]
return cli
func (clients *ClientManager) removeInternal(client *Client) (removed bool) {
// requires holding ByNickMutex
oldcfnick := client.NickCasefolded()
currentEntry, present := clients.byNick[oldcfnick]
if present {
if currentEntry == client {
delete(clients.byNick, oldcfnick)
removed = true
} else {
// this shouldn't happen, but we can ignore it
client.server.logger.Warning("internal", fmt.Sprintf("clients for nick %s out of sync", oldcfnick))
}
return nil
}
// Add adds a client to the lookup set.
func (clients *ClientLookupSet) Add(client *Client, nick string) error {
nick, err := CasefoldName(nick)
if err != nil {
return err
}
clients.ByNickMutex.Lock()
defer clients.ByNickMutex.Unlock()
if clients.getNoMutex(nick) != nil {
return ErrNicknameInUse
}
clients.ByNick[nick] = client
return nil
return
}
// Remove removes a client from the lookup set.
func (clients *ClientLookupSet) Remove(client *Client) error {
func (clients *ClientManager) Remove(client *Client) error {
clients.Lock()
defer clients.Unlock()
if !client.HasNick() {
return ErrNickMissing
}
clients.ByNickMutex.Lock()
defer clients.ByNickMutex.Unlock()
if clients.getNoMutex(client.nick) != client {
return ErrNicknameMismatch
}
delete(clients.ByNick, client.nickCasefolded)
clients.removeInternal(client)
return nil
}
// Replace renames an existing client in the lookup set.
func (clients *ClientLookupSet) Replace(oldNick, newNick string, client *Client) error {
// get casefolded nicknames
oldNick, err := CasefoldName(oldNick)
if err != nil {
return err
}
newNick, err = CasefoldName(newNick)
// SetNick sets a client's nickname, validating it against nicknames in use
func (clients *ClientManager) SetNick(client *Client, newNick string) error {
newcfnick, err := CasefoldName(newNick)
if err != nil {
return err
}
// remove and replace
clients.ByNickMutex.Lock()
defer clients.ByNickMutex.Unlock()
clients.Lock()
defer clients.Unlock()
oldClient := clients.ByNick[newNick]
if oldClient == nil || oldClient == client {
// whoo
} else {
clients.removeInternal(client)
currentNewEntry := clients.byNick[newcfnick]
// the client may just be changing case
if currentNewEntry != nil && currentNewEntry != client {
return ErrNicknameInUse
}
if oldNick == newNick {
// if they're only changing case, don't need to remove+re-add them
clients.byNick[newcfnick] = client
client.updateNickMask(newNick)
return nil
}
func (clients *ClientManager) AllClients() (result []*Client) {
clients.RLock()
defer clients.RUnlock()
result = make([]*Client, len(clients.byNick))
i := 0
for _, client := range(clients.byNick) {
result[i] = client
i++
}
delete(clients.ByNick, oldNick)
clients.ByNick[newNick] = client
return nil
return
}
// AllWithCaps returns all clients with the given capabilities.
func (clients *ClientLookupSet) AllWithCaps(capabs ...caps.Capability) (set ClientSet) {
func (clients *ClientManager) AllWithCaps(capabs ...caps.Capability) (set ClientSet) {
set = make(ClientSet)
clients.ByNickMutex.RLock()
defer clients.ByNickMutex.RUnlock()
clients.RLock()
defer clients.RUnlock()
var client *Client
for _, client = range clients.ByNick {
for _, client = range clients.byNick {
// make sure they have all the required caps
for _, capab := range capabs {
if !client.capabilities.Has(capab) {
@ -177,7 +152,7 @@ func (clients *ClientLookupSet) AllWithCaps(capabs ...caps.Capability) (set Clie
}
// FindAll returns all clients that match the given userhost mask.
func (clients *ClientLookupSet) FindAll(userhost string) (set ClientSet) {
func (clients *ClientManager) FindAll(userhost string) (set ClientSet) {
set = make(ClientSet)
userhost, err := Casefold(ExpandUserHost(userhost))
@ -186,9 +161,9 @@ func (clients *ClientLookupSet) FindAll(userhost string) (set ClientSet) {
}
matcher := ircmatch.MakeMatch(userhost)
clients.ByNickMutex.RLock()
defer clients.ByNickMutex.RUnlock()
for _, client := range clients.ByNick {
clients.RLock()
defer clients.RUnlock()
for _, client := range clients.byNick {
if matcher.Match(client.nickMaskCasefolded) {
set.Add(client)
}
@ -198,7 +173,7 @@ func (clients *ClientLookupSet) FindAll(userhost string) (set ClientSet) {
}
// Find returns the first client that matches the given userhost mask.
func (clients *ClientLookupSet) Find(userhost string) *Client {
func (clients *ClientManager) Find(userhost string) *Client {
userhost, err := Casefold(ExpandUserHost(userhost))
if err != nil {
return nil
@ -206,9 +181,9 @@ func (clients *ClientLookupSet) Find(userhost string) *Client {
matcher := ircmatch.MakeMatch(userhost)
var matchedClient *Client
clients.ByNickMutex.RLock()
defer clients.ByNickMutex.RUnlock()
for _, client := range clients.ByNick {
clients.RLock()
defer clients.RUnlock()
for _, client := range clients.byNick {
if matcher.Match(client.nickMaskCasefolded) {
matchedClient = client
break

6
irc/dline.go
View File

@ -82,7 +82,7 @@ type dLineNet struct {
// DLineManager manages and dlines.
type DLineManager struct {
sync.RWMutex
sync.RWMutex // tier 1
// addresses that are dlined
addresses map[string]*dLineAddr
// networks that are dlined
@ -386,8 +386,7 @@ func dlineHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
var killedClientNicks []string
var toKill bool
server.clients.ByNickMutex.RLock()
for _, mcl := range server.clients.ByNick {
for _, mcl := range server.clients.AllClients() {
if hostNet == nil {
toKill = hostAddr.Equal(mcl.IP())
} else {
@ -399,7 +398,6 @@ func dlineHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
killedClientNicks = append(killedClientNicks, mcl.nick)
}
}
server.clients.ByNickMutex.RUnlock()
for _, mcl := range clientsToKill {
mcl.exitedSnomaskSent = true

2
irc/idletimer.go
View File

@ -29,7 +29,7 @@ const (
)
type IdleTimer struct {
sync.Mutex
sync.Mutex // tier 1
// immutable after construction
registerTimeout time.Duration

6
irc/kline.go
View File

@ -35,7 +35,7 @@ type KLineInfo struct {
// KLineManager manages and klines.
type KLineManager struct {
sync.RWMutex
sync.RWMutex // tier 1
// kline'd entries
entries map[string]*KLineInfo
}
@ -282,8 +282,7 @@ func klineHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
var clientsToKill []*Client
var killedClientNicks []string
server.clients.ByNickMutex.RLock()
for _, mcl := range server.clients.ByNick {
for _, mcl := range server.clients.AllClients() {
for _, clientMask := range mcl.AllNickmasks() {
if matcher.Match(clientMask) {
clientsToKill = append(clientsToKill, mcl)
@ -291,7 +290,6 @@ func klineHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
}
}
}
server.clients.ByNickMutex.RUnlock()
for _, mcl := range clientsToKill {
mcl.exitedSnomaskSent = true

2
irc/monitor.go
View File

@ -15,7 +15,7 @@ import (
// MonitorManager keeps track of who's monitoring which nicks.
type MonitorManager struct {
sync.RWMutex
sync.RWMutex // tier 2
// client -> nicks it's watching
watching map[*Client]map[string]bool
// nick -> clients watching it

72
irc/nickname.go
View File

@ -8,7 +8,9 @@ import (
"fmt"
"strings"
"github.com/goshuirc/irc-go/ircfmt"
"github.com/goshuirc/irc-go/ircmsg"
"github.com/oragono/oragono/irc/sno"
)
var (
@ -26,7 +28,11 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
return true
}
nicknameRaw := strings.TrimSpace(msg.Params[0])
return performNickChange(server, client, client, msg.Params[0])
}
func performNickChange(server *Server, client *Client, target *Client, newnick string) bool {
nicknameRaw := strings.TrimSpace(newnick)
nickname, err := CasefoldName(nicknameRaw)
if len(nicknameRaw) < 1 {
@ -39,16 +45,14 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
return false
}
if client.nick == nickname {
if target.Nick() == nicknameRaw {
return false
}
// bleh, this will be replaced and done below
if client.registered {
err = client.ChangeNickname(nicknameRaw)
} else {
err = client.SetNickname(nicknameRaw)
}
hadNick := target.HasNick()
origNick := target.Nick()
origNickMask := target.NickMaskString()
err = client.server.clients.SetNick(target, nickname)
if err == ErrNicknameInUse {
client.Send(nil, server.name, ERR_NICKNAMEINUSE, client.nick, nicknameRaw, "Nickname is already in use")
return false
@ -56,49 +60,31 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
client.Send(nil, server.name, ERR_UNKNOWNERROR, client.nick, "NICK", fmt.Sprintf("Could not set or change nickname: %s", err.Error()))
return false
}
if client.registered {
client.server.monitorManager.AlertAbout(client, true)
client.server.logger.Debug("nick", fmt.Sprintf("%s changed nickname to %s", origNickMask, nickname))
if hadNick {
target.server.snomasks.Send(sno.LocalNicks, fmt.Sprintf(ircfmt.Unescape("$%s$r changed nickname to %s"), origNick, nicknameRaw))
target.server.whoWas.Append(client)
for friend := range target.Friends() {
friend.Send(nil, origNickMask, "NICK", nicknameRaw)
}
}
if target.registered {
client.server.monitorManager.AlertAbout(target, true)
} else {
server.tryRegister(target)
}
server.tryRegister(client)
return false
}
// SANICK <oldnick> <nickname>
func sanickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
if !client.authorized {
client.Quit("Bad password")
return true
}
oldnick, oerr := CasefoldName(msg.Params[0])
nickname, err := CasefoldName(msg.Params[1])
if len(nickname) < 1 {
client.Send(nil, server.name, ERR_NONICKNAMEGIVEN, client.nick, "No nickname given")
return false
}
if oerr != nil || err != nil || len(strings.TrimSpace(msg.Params[1])) > server.limits.NickLen || restrictedNicknames[nickname] {
client.Send(nil, server.name, ERR_ERRONEUSNICKNAME, client.nick, msg.Params[0], "Erroneous nickname")
return false
}
if client.nick == msg.Params[1] {
return false
}
target := server.clients.Get(oldnick)
targetNick := strings.TrimSpace(msg.Params[0])
target := server.clients.Get(targetNick)
if target == nil {
client.Send(nil, server.name, ERR_NOSUCHNICK, client.nick, msg.Params[0], "No such nick")
return false
}
//TODO(dan): There's probably some races here, we should be changing this in the primary server thread
if server.clients.Get(nickname) != nil && server.clients.Get(nickname) != target {
client.Send(nil, server.name, ERR_NICKNAMEINUSE, client.nick, msg.Params[0], "Nickname is already in use")
return false
}
target.ChangeNickname(msg.Params[1])
return false
return performNickChange(server, client, target, msg.Params[1])
}

27
irc/server.go
View File

@ -74,7 +74,7 @@ type ListenerWrapper struct {
// lets the ListenerWrapper inform the server that it has stopped:
stopEvent chan bool
// protects atomic update of tlsConfig and shouldStop:
configMutex sync.Mutex
configMutex sync.Mutex // tier 1
}
// Server is the main Oragono server.
@ -86,10 +86,10 @@ type Server struct {
channels *ChannelManager
channelRegistry *ChannelRegistry
checkIdent bool
clients *ClientLookupSet
clients *ClientManager
commands chan Command
configFilename string
configurableStateMutex sync.RWMutex // generic protection for server state modified by rehash()
configurableStateMutex sync.RWMutex // tier 1; generic protection for server state modified by rehash()
connectionLimiter *connection_limits.Limiter
connectionThrottler *connection_limits.Throttler
ctime time.Time
@ -113,7 +113,7 @@ type Server struct {
password []byte
passwords *passwd.SaltedManager
recoverFromErrors bool
rehashMutex sync.Mutex
rehashMutex sync.Mutex // tier 3
rehashSignal chan os.Signal
proxyAllowedFrom []string
signals chan os.Signal
@ -149,7 +149,7 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) {
server := &Server{
accounts: make(map[string]*ClientAccount),
channels: NewChannelManager(),
clients: NewClientLookupSet(),
clients: NewClientManager(),
commands: make(chan Command),
connectionLimiter: connection_limits.NewLimiter(),
connectionThrottler: connection_limits.NewThrottler(),
@ -238,11 +238,9 @@ func loadChannelList(channel *Channel, list string, maskMode Mode) {
// Shutdown shuts down the server.
func (server *Server) Shutdown() {
//TODO(dan): Make sure we disallow new nicks
server.clients.ByNickMutex.RLock()
for _, client := range server.clients.ByNick {
for _, client := range server.clients.AllClients() {
client.Notice("Server is shutting down")
}
server.clients.ByNickMutex.RUnlock()
if err := server.store.Close(); err != nil {
server.logger.Error("shutdown", fmt.Sprintln("Could not close datastore:", err))
@ -1332,11 +1330,9 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
server.configurableStateMutex.Unlock()
// update on all clients
server.clients.ByNickMutex.RLock()
for _, sClient := range server.clients.ByNick {
for _, sClient := range server.clients.AllClients() {
sClient.socket.MaxSendQBytes = config.Server.MaxSendQBytes
}
server.clients.ByNickMutex.RUnlock()
}
// set RPL_ISUPPORT
@ -1370,8 +1366,7 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
if !initial {
// push new info to all of our clients
server.clients.ByNickMutex.RLock()
for _, sClient := range server.clients.ByNick {
for _, sClient := range server.clients.AllClients() {
for _, tokenline := range newISupportReplies {
sClient.Send(nil, server.name, RPL_ISUPPORT, append([]string{sClient.nick}, tokenline...)...)
}
@ -1380,7 +1375,6 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
sClient.Notice(rawIONotice)
}
}
server.clients.ByNickMutex.RUnlock()
}
return nil
@ -2008,10 +2002,7 @@ func lusersHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
//TODO(vegax87) Fix network statistics and additional parameters
var totalcount, invisiblecount, opercount int
server.clients.ByNickMutex.RLock()
defer server.clients.ByNickMutex.RUnlock()
for _, onlineusers := range server.clients.ByNick {
for _, onlineusers := range server.clients.AllClients() {
totalcount++
if onlineusers.flags[Invisible] {
invisiblecount++

2
irc/snomanager.go
View File

@ -10,7 +10,7 @@ import (
// SnoManager keeps track of which clients to send snomasks to.
type SnoManager struct {
sendListMutex sync.RWMutex
sendListMutex sync.RWMutex // tier 2
sendLists map[sno.Mask]map[*Client]bool
}

2
irc/whowas.go
View File

@ -14,7 +14,7 @@ type WhoWasList struct {
start int
end int
accessMutex sync.RWMutex
accessMutex sync.RWMutex // tier 2
}
// WhoWas is an entry in the WhoWasList.