Skip to content

Commit

Permalink
Fix incorrect threading issues when using Matterircd's default threading
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.
  • Loading branch information
hloeung committed Sep 6, 2023
1 parent 6dff4fb commit bfdad41
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 31 deletions.
39 changes: 12 additions & 27 deletions mm-go-irckit/server_commands.go
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
45 changes: 41 additions & 4 deletions mm-go-irckit/userbridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ type UserBridge struct {
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 @@ -62,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 @@ -913,16 +916,49 @@ func (u *User) logoutFrom(protocol string) error {
func (u *User) increaseMsgCounter(channelID string) int {
u.msgCounterMutex.Lock()
defer u.msgCounterMutex.Unlock()

u.msgCounter[channelID]++

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

if msgID == messageID {
return
}

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

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

Check failure on line 959 in mm-go-irckit/userbridge.go

View workflow job for this annotation

GitHub Actions / golangci-lint

S1023: redundant `return` statement (gosimple)
}

func (u *User) formatContextMessage(ts, threadMsgID, msg string) string {
var formattedMsg string
switch {
Expand Down Expand Up @@ -971,18 +1007,19 @@ func (u *User) prefixContext(channelID, messageID, parentID, event string) strin
}

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

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

currentcount = u.msgMap[channelID][messageID]
} else {
u.msgMap[channelID][messageID] = u.increaseMsgCounter(channelID)
currentcount = u.msgMap[channelID][messageID]
}
u.updateMsgMapIndex(channelID, currentcount, messageID)

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

0 comments on commit bfdad41

Please sign in to comment.