From 45c44aee50db5991d468504be08c16a3a62207fb Mon Sep 17 00:00:00 2001 From: David Hill Date: Mon, 5 Nov 2018 18:39:28 -0500 Subject: [PATCH] [slack]: cleanup previous commit - move global vars to struct - use defer, which fixes refreshInProgress being set while unlocked --- bridge/slack/helpers.go | 47 ++++++++++++++++++++--------------------- bridge/slack/slack.go | 20 ++++++++++++------ 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/bridge/slack/helpers.go b/bridge/slack/helpers.go index 4abe79cd..23d5816a 100644 --- a/bridge/slack/helpers.go +++ b/bridge/slack/helpers.go @@ -4,7 +4,6 @@ import ( "fmt" "regexp" "strings" - "sync" "time" "github.com/nlopes/slack" @@ -61,22 +60,17 @@ func (b *Bslack) getChannelByID(ID string) (*slack.Channel, error) { const minimumRefreshInterval = 10 * time.Second -var ( - refreshMutex sync.Mutex - refreshInProgress bool - earliestChannelRefresh = time.Now() - earliestUserRefresh = time.Now() -) - func (b *Bslack) populateUsers() { - refreshMutex.Lock() - if time.Now().Before(earliestUserRefresh) || refreshInProgress { - b.Log.Debugf("Not refreshing user list as it was done less than %d seconds ago.", int(minimumRefreshInterval.Seconds())) - refreshMutex.Unlock() + b.refreshMutex.Lock() + if time.Now().Before(b.earliestUserRefresh) || b.refreshInProgress { + b.Log.Debugf("Not refreshing user list as it was done less than %v ago.", + minimumRefreshInterval) + b.refreshMutex.Unlock() + return } - refreshInProgress = true - refreshMutex.Unlock() + b.refreshInProgress = true + b.refreshMutex.Unlock() users, err := b.sc.GetUsers() if err != nil { @@ -95,19 +89,22 @@ func (b *Bslack) populateUsers() { defer b.usersMutex.Unlock() b.users = newUsers - earliestUserRefresh = time.Now().Add(minimumRefreshInterval) - refreshInProgress = false + b.refreshMutex.Lock() + defer b.refreshMutex.Unlock() + b.earliestUserRefresh = time.Now().Add(minimumRefreshInterval) + b.refreshInProgress = false } func (b *Bslack) populateChannels() { - refreshMutex.Lock() - if time.Now().Before(earliestChannelRefresh) || refreshInProgress { - b.Log.Debugf("Not refreshing channel list as it was done less than %d seconds ago.", int(minimumRefreshInterval.Seconds())) - refreshMutex.Unlock() + b.refreshMutex.Lock() + if time.Now().Before(b.earliestChannelRefresh) || b.refreshInProgress { + b.Log.Debugf("Not refreshing channel list as it was done less than %v seconds ago.", + minimumRefreshInterval) + b.refreshMutex.Unlock() return } - refreshInProgress = true - refreshMutex.Unlock() + b.refreshInProgress = true + b.refreshMutex.Unlock() newChannelsByID := map[string]*slack.Channel{} newChannelsByName := map[string]*slack.Channel{} @@ -139,8 +136,10 @@ func (b *Bslack) populateChannels() { b.channelsByID = newChannelsByID b.channelsByName = newChannelsByName - earliestChannelRefresh = time.Now().Add(minimumRefreshInterval) - refreshInProgress = false + b.refreshMutex.Lock() + defer b.refreshMutex.Unlock() + b.earliestChannelRefresh = time.Now().Add(minimumRefreshInterval) + b.refreshInProgress = false } var ( diff --git a/bridge/slack/slack.go b/bridge/slack/slack.go index 924f4131..c1386a46 100644 --- a/bridge/slack/slack.go +++ b/bridge/slack/slack.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" "sync" + "time" "github.com/42wim/matterbridge/bridge" "github.com/42wim/matterbridge/bridge/config" @@ -34,6 +35,11 @@ type Bslack struct { channelsByID map[string]*slack.Channel channelsByName map[string]*slack.Channel channelsMutex sync.RWMutex + + refreshInProgress bool + earliestChannelRefresh time.Time + earliestUserRefresh time.Time + refreshMutex sync.Mutex } const ( @@ -68,12 +74,14 @@ func New(cfg *bridge.Config) bridge.Bridger { cfg.Log.Fatalf("Could not create LRU cache for Slack bridge: %v", err) } b := &Bslack{ - Config: cfg, - uuid: xid.New().String(), - cache: newCache, - users: map[string]*slack.User{}, - channelsByID: map[string]*slack.Channel{}, - channelsByName: map[string]*slack.Channel{}, + Config: cfg, + uuid: xid.New().String(), + cache: newCache, + users: map[string]*slack.User{}, + channelsByID: map[string]*slack.Channel{}, + channelsByName: map[string]*slack.Channel{}, + earliestChannelRefresh: time.Now(), + earliestUserRefresh: time.Now(), } return b }