From a74450d6caede853a26df8c632107a29d2b521e2 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 01:22:00 -0500 Subject: [PATCH 1/7] remove redundant database write on always-on recreation --- irc/channel.go | 2 +- irc/client.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index 3b241435..ba8658bd 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -720,7 +720,7 @@ func (channel *Channel) Join(client *Client, key string, isSajoin bool, rb *Resp channel.AddHistoryItem(histItem) } - client.addChannel(channel) + client.addChannel(channel, rb == nil) if rb == nil { return diff --git a/irc/client.go b/irc/client.go index 9e1e3ff3..6841791f 100644 --- a/irc/client.go +++ b/irc/client.go @@ -1496,13 +1496,15 @@ func (session *Session) Notice(text string) { session.Send(nil, session.client.server.name, "NOTICE", session.client.Nick(), text) } -func (client *Client) addChannel(channel *Channel) { +// `simulated` is for the fake join of an always-on client +// (we just read the channel name from the database, there's no need to write it back) +func (client *Client) addChannel(channel *Channel, simulated bool) { client.stateMutex.Lock() client.channels[channel] = true alwaysOn := client.alwaysOn client.stateMutex.Unlock() - if alwaysOn { + if alwaysOn && !simulated { client.markDirty(IncludeChannels) } } From c0192e0e526253314788f43bdfe56d5381e65d7d Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 01:32:08 -0500 Subject: [PATCH 2/7] add missing initialization for writerSemaphore It was only initialized for always-on clients, not for regular clients. This explains a lot in terms of #812 failing to reproduce. --- irc/client.go | 1 + 1 file changed, 1 insertion(+) diff --git a/irc/client.go b/irc/client.go index 6841791f..2d920786 100644 --- a/irc/client.go +++ b/irc/client.go @@ -263,6 +263,7 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) { nickCasefolded: "*", nickMaskString: "*", // * is used until actual nick is given } + client.writerSemaphore.Initialize(1) client.history.Initialize(config.History.ClientLength, config.History.AutoresizeWindow) client.brbTimer.Initialize(client) session := &Session{ From 3005e95c1fde9e09ae409c1f19fd677e3613fec3 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 01:46:22 -0500 Subject: [PATCH 3/7] rename IncludeAllChannelAttrs --- irc/channel.go | 4 ++-- irc/channelmanager.go | 4 ++-- irc/channelreg.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index ba8658bd..3958355f 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -359,7 +359,7 @@ func (channel *Channel) Transfer(client *Client, target string, hasPrivs bool) ( status = channelTransferFailed defer func() { if status == channelTransferComplete && err == nil { - channel.Store(IncludeAllChannelAttrs) + channel.Store(IncludeAllAttrs) } }() @@ -400,7 +400,7 @@ func (channel *Channel) transferOwnership(newOwner string) { func (channel *Channel) AcceptTransfer(client *Client) (err error) { defer func() { if err == nil { - channel.Store(IncludeAllChannelAttrs) + channel.Store(IncludeAllAttrs) } }() diff --git a/irc/channelmanager.go b/irc/channelmanager.go index dc711e55..9312313c 100644 --- a/irc/channelmanager.go +++ b/irc/channelmanager.go @@ -198,7 +198,7 @@ func (cm *ChannelManager) SetRegistered(channelName string, account string) (err defer func() { if err == nil && channel != nil { // registration was successful: make the database reflect it - err = channel.Store(IncludeAllChannelAttrs) + err = channel.Store(IncludeAllAttrs) } }() @@ -272,7 +272,7 @@ func (cm *ChannelManager) Rename(name string, newName string) (err error) { var info RegisteredChannel defer func() { if channel != nil && info.Founder != "" { - channel.Store(IncludeAllChannelAttrs) + channel.Store(IncludeAllAttrs) // we just flushed the channel under its new name, therefore this delete // cannot be overwritten by a write to the old name: cm.server.channelRegistry.Delete(info) diff --git a/irc/channelreg.go b/irc/channelreg.go index 984bd9fd..c1140538 100644 --- a/irc/channelreg.go +++ b/irc/channelreg.go @@ -70,7 +70,7 @@ const ( // this is an OR of all possible flags const ( - IncludeAllChannelAttrs = ^uint(0) + IncludeAllAttrs = ^uint(0) ) // RegisteredChannel holds details about a given registered channel. From d5f68215e19f9f6960da888b2b2c828f205c911b Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 01:53:02 -0500 Subject: [PATCH 4/7] mark dirty when a client first becomes always-on --- irc/getters.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/irc/getters.go b/irc/getters.go index cd1a75d1..a74a44da 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -321,13 +321,19 @@ func (client *Client) AccountSettings() (result AccountSettings) { } func (client *Client) SetAccountSettings(settings AccountSettings) { + // we mark dirty if the client is transitioning to always-on + markDirty := false alwaysOn := persistenceEnabled(client.server.Config().Accounts.Multiclient.AlwaysOn, settings.AlwaysOn) client.stateMutex.Lock() client.accountSettings = settings if client.registered { + markDirty = !client.alwaysOn && alwaysOn client.alwaysOn = alwaysOn } client.stateMutex.Unlock() + if markDirty { + client.markDirty(IncludeAllAttrs) + } } func (client *Client) Languages() (languages []string) { From e7c1800893c7d25dc8c0ba174d042aabdaabc04a Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 01:54:40 -0500 Subject: [PATCH 5/7] fix a spurious error logline unregistering an always-on client would produce "attempting to persist logged-out client : x" because the client was always-on, but also being ejected --- irc/accounts.go | 1 + 1 file changed, 1 insertion(+) diff --git a/irc/accounts.go b/irc/accounts.go index b9082083..63371d6f 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -1152,6 +1152,7 @@ func (am *AccountManager) Unregister(account string) error { } for _, client := range clients { if config.Accounts.RequireSasl.Enabled { + client.Logout() client.Quit(client.t("You are no longer authorized to be on this server"), nil) // destroy acquires a semaphore so we can't call it while holding a lock go client.destroy(nil) From d72037725b5d7f4fbf114dc9b2ff099f4b10f10e Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 02:52:51 -0500 Subject: [PATCH 6/7] simplify read of lastSeen --- irc/client.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/irc/client.go b/irc/client.go index 2d920786..fab3d6a1 100644 --- a/irc/client.go +++ b/irc/client.go @@ -1624,6 +1624,7 @@ func (client *Client) performWrite() { dirtyBits := client.dirtyBits client.dirtyBits = 0 account := client.account + lastSeen := client.lastSeen client.stateMutex.Unlock() if account == "" { @@ -1640,9 +1641,6 @@ func (client *Client) performWrite() { client.server.accounts.saveChannels(account, channelNames) } if (dirtyBits & IncludeLastSeen) != 0 { - client.stateMutex.RLock() - lastSeen := client.lastSeen - client.stateMutex.RUnlock() client.server.accounts.saveLastSeen(account, lastSeen) } } From 5447fc79ffe303195da3a934dc2c71bc011dcf6b Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Mar 2020 03:06:57 -0500 Subject: [PATCH 7/7] fix confusion between lastSeen and lastActive --- irc/client.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/irc/client.go b/irc/client.go index fab3d6a1..58be9196 100644 --- a/irc/client.go +++ b/irc/client.go @@ -308,16 +308,16 @@ func (server *Server) RunClient(conn clientConn, proxyLine string) { client.run(session, proxyLine) } -func (server *Server) AddAlwaysOnClient(account ClientAccount, chnames []string, lastActive time.Time) { +func (server *Server) AddAlwaysOnClient(account ClientAccount, chnames []string, lastSeen time.Time) { now := time.Now().UTC() config := server.Config() - if lastActive.IsZero() { - lastActive = now + if lastSeen.IsZero() { + lastSeen = now } client := &Client{ - lastSeen: now, - lastActive: lastActive, + lastSeen: lastSeen, + lastActive: now, channels: make(ChannelSet), ctime: now, languages: server.Languages().Default(),