Skip to content

Commit

Permalink
Fix incorrect msg/thread counters (#542)
Browse files Browse the repository at this point in the history 
When using mattericd's default threading, the msgMap is populated with
message/thred IDs and never removed. This means that when we cycle
past 4k, we can have entries with duplicate counters causing issues
when replying to threads.

This fixes it by introducing an index.

Also, rework the way Matterircd threading works.
  • Loading branch information
@hloeung
hloeung authored Sep 10, 2023
1 parent 7f4e2d6 commit 8adbbd3
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 73 deletions.
39 changes: 12 additions & 27 deletions mm-go-irckit/server_commands.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,16 +478,11 @@ func parseReactionToMsg(u *User, msg *irc.Message, channelID string) bool {
logger.Errorf("couldn't parseint %s: %s", msgID, err)
}

u.msgMapMutex.RLock()
defer u.msgMapMutex.RUnlock()
u.msgMapIndexMutex.RLock()
defer u.msgMapIndexMutex.RUnlock()

m := u.msgMap[channelID]

for k, v := range m {
if v == int(id) {
msgID = k
break
}
if _, ok := u.msgMapIndex[channelID][int(id)]; ok {
msgID = u.msgMapIndex[channelID][int(id)]
}
}

Expand Down Expand Up @@ -549,17 +544,11 @@ func parseModifyMsg(u *User, msg *irc.Message, channelID string) bool {
logger.Errorf("couldn't parseint %s: %s", matches[1], err)
}

u.msgMapMutex.RLock()
defer u.msgMapMutex.RUnlock()
u.msgMapIndexMutex.RLock()
defer u.msgMapIndexMutex.RUnlock()

m := u.msgMap[channelID]

for k, v := range m {
if v != int(id) {
continue
}

msgID = k
if _, ok := u.msgMapIndex[channelID][int(id)]; ok {
msgID = u.msgMapIndex[channelID][int(id)]

u.msgLastMutex.Lock()
defer u.msgLastMutex.Unlock()
Expand Down Expand Up @@ -619,15 +608,11 @@ func parseThreadID(u *User, msg *irc.Message, channelID string) (string, string)
return "", ""
}

u.msgMapMutex.RLock()
defer u.msgMapMutex.RUnlock()

m := u.msgMap[channelID]
u.msgMapIndexMutex.RLock()
defer u.msgMapIndexMutex.RUnlock()

for k, v := range m {
if v == int(id) {
return k, matches[2]
}
if _, ok := u.msgMapIndex[channelID][int(id)]; ok {
return u.msgMapIndex[channelID][int(id)], matches[2]
}
case len(matches[1]) == 26:
return matches[1], matches[2]
Expand Down
121 changes: 75 additions & 46 deletions mm-go-irckit/userbridge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,18 @@ type UserBridge struct {
eventChan chan *bridge.Event //nolint:structcheck
away bool //nolint:structcheck

lastViewedAtDB *bolt.DB //nolint:structcheck
msgCounter map[string]int //nolint:structcheck
lastViewedAtDB *bolt.DB //nolint:structcheck

msgCounterMutex sync.RWMutex //nolint:structcheck
msgCounter map[string]int //nolint:structcheck

msgLastMutex sync.RWMutex //nolint:structcheck
msgLast map[string][2]string //nolint:structcheck

msgMapMutex sync.RWMutex //nolint:structcheck
msgMap map[string]map[string]int //nolint:structcheck
msgMapMutex sync.RWMutex //nolint:structcheck
msgMap map[string]map[string]int //nolint:structcheck
msgMapIndexMutex sync.RWMutex //nolint:structcheck
msgMapIndex map[string]map[int]string //nolint:structcheck

updateCounterMutex sync.Mutex //nolint:structcheck
updateCounter map[string]time.Time //nolint:structcheck
Expand All @@ -60,6 +64,7 @@ func NewUserBridge(c net.Conn, srv Server, cfg *viper.Viper, db *bolt.DB) *User
u.lastViewedAtDB = db
u.msgLast = make(map[string][2]string)
u.msgMap = make(map[string]map[string]int)
u.msgMapIndex = make(map[string]map[int]string)
u.msgCounter = make(map[string]int)
u.updateCounter = make(map[string]time.Time)
u.eventChan = make(chan *bridge.Event, 1000)
Expand Down Expand Up @@ -908,17 +913,60 @@ func (u *User) logoutFrom(protocol string) error {
return nil
}

func (u *User) increaseMsgCounter(channelID string) int {
func (u *User) increaseMsgCounter(channelID string, skip int) int {
u.msgCounterMutex.Lock()
defer u.msgCounterMutex.Unlock()

u.msgCounter[channelID]++

// max 4096 entries
if u.msgCounter[channelID] == 4095 {
u.msgCounter[channelID] = 0
// max 4096 entries (0xFFF); set back to 1, 0 is used for absent.
if u.msgCounter[channelID] >= 4096 {
u.msgCounter[channelID] = 1
}

if skip != 0 && u.msgCounter[channelID] == skip {
u.msgCounter[channelID]++

// max 4096 entries (0xFFF); set back to 1, 0 is used for absent.
if u.msgCounter[channelID] >= 4096 {
u.msgCounter[channelID] = 1
}
}

return u.msgCounter[channelID]
}

func (u *User) updateMsgMapIndex(channelID string, counter int, messageID string) {
u.msgMapIndexMutex.Lock()
defer u.msgMapIndexMutex.Unlock()

var (
ok bool
msgID string
)

if _, ok = u.msgMapIndex[channelID]; !ok {
u.msgMapIndex[channelID] = make(map[int]string)
}

if msgID, ok = u.msgMapIndex[channelID][counter]; !ok {
u.msgMapIndex[channelID][counter] = messageID
return
}

// map entry is the same as the one provided so do nothing.
if msgID == messageID {
return
}

// Remove previous msgID from MsgMap with the same counter.
if _, ok = u.msgMap[channelID][msgID]; ok {
delete(u.msgMap[channelID], msgID)
}

u.msgMapIndex[channelID][counter] = messageID
}

func (u *User) formatContextMessage(ts, threadMsgID, msg string) string {
var formattedMsg string
switch {
Expand All @@ -933,34 +981,16 @@ func (u *User) formatContextMessage(ts, threadMsgID, msg string) string {
return formattedMsg
}

func (u *User) prefixContextModified(channelID, messageID string) string {
var (
ok bool
currentcount int
)

if _, ok = u.msgMap[channelID]; !ok {
u.msgMap[channelID] = make(map[string]int)
}

// check if we already have a counter for this messageID otherwise
// increase counter and create it
if currentcount, ok = u.msgMap[channelID][messageID]; !ok {
currentcount = u.increaseMsgCounter(channelID)
func (u *User) prefixContext(channelID, messageID, parentID, event string) string {
prefixChar := "->"
if u.v.GetBool(u.br.Protocol() + ".unicode") {
prefixChar = "↪"
}

return fmt.Sprintf("[%03x]", currentcount)
}

func (u *User) prefixContext(channelID, messageID, parentID, event string) string {
if u.v.GetString(u.br.Protocol()+".threadcontext") == "mattermost" || u.v.GetString(u.br.Protocol()+".threadcontext") == "mattermost+post" {
if parentID == "" {
return fmt.Sprintf("[@@%s]", messageID)
}
prefixChar := "->"
if u.v.GetBool(u.br.Protocol() + ".unicode") {
prefixChar = "↪"
}
if u.v.GetString(u.br.Protocol()+".threadcontext") == "mattermost" || parentID == messageID {
return fmt.Sprintf("[%s@@%s]", prefixChar, parentID)
}
Expand All @@ -970,38 +1000,37 @@ func (u *User) prefixContext(channelID, messageID, parentID, event string) strin
u.msgMapMutex.Lock()
defer u.msgMapMutex.Unlock()

if event == "post_edited" || event == "post_deleted" || event == "reaction" {
return u.prefixContextModified(channelID, messageID)
}

var (
currentcount, parentcount int
ok bool
)

if parentID != "" {
if _, ok = u.msgMap[channelID]; !ok {
u.msgMap[channelID] = make(map[string]int)
}
if _, ok = u.msgMap[channelID]; !ok {
u.msgMap[channelID] = make(map[string]int)
}

if parentID != "" {
if _, ok = u.msgMap[channelID][parentID]; !ok {
u.increaseMsgCounter(channelID)
u.msgMap[channelID][parentID] = u.msgCounter[channelID]
u.msgMap[channelID][parentID] = u.increaseMsgCounter(channelID, parentcount)
}

parentcount = u.msgMap[channelID][parentID]
u.updateMsgMapIndex(channelID, parentcount, parentID)
}

currentcount = u.increaseMsgCounter(channelID)

if _, ok = u.msgMap[channelID]; !ok {
u.msgMap[channelID] = make(map[string]int)
if event == "post_edited" || event == "post_deleted" || event == "reaction" {
if _, ok = u.msgMap[channelID][messageID]; !ok {
u.msgMap[channelID][messageID] = u.increaseMsgCounter(channelID, parentcount)
}
} else {
u.msgMap[channelID][messageID] = u.increaseMsgCounter(channelID, parentcount)
}

u.msgMap[channelID][messageID] = u.msgCounter[channelID]
currentcount = u.msgMap[channelID][messageID]
u.updateMsgMapIndex(channelID, currentcount, messageID)

if parentID != "" {
return fmt.Sprintf("[%03x->%03x]", currentcount, parentcount)
return fmt.Sprintf("[%s%03x,%03x]", prefixChar, parentcount, currentcount)
}

return fmt.Sprintf("[%03x]", currentcount)
Expand Down

0 comments on commit 8adbbd3

Please sign in to comment.