Do I need a mutex in this case? I am refreshing the token with a goroutine, the token is used in another goroutine. In other words, will my token be empty at some point so that the response will be a 401?
If yes, is it part of the structure c *threatq or is it a simple variable, I mean, a "standalone" one inside my code.
// IndicatorChannelIterator returns indicators from ThreatQ into a channel.
func (c *threatq) IndicatorChannelIterator() (<-chan *models.Indicator, error) {
// Authenticate
token, err := c.authenticate(c.clientID, c.email, c.password)
if err != nil {
return nil, fmt.Errorf("Error while authenticating to TQ : %s", err)
}
// Periodically refresh the token
ticker := time.NewTicker(30 * time.Minute)
go func() {
for range ticker.C {
token, err = c.authenticate(c.clientID, c.email, c.password)
if err != nil {
logrus.Errorf("Error while authenticating to TQ : %s", err)
}
}
}()
// Prepare the query
query := &Query{}
// Get the first page
firstTQResponse, err := c.advancedSearch(query, token, 0)
if err != nil {
return nil, fmt.Errorf("Error while getting the first page from TQ : %s", err)
}
// Create the channel
indicators := make(chan *models.Indicator)
// Request the others
go func() {
req := 1
total := firstTQResponse.Total
for offset := 0; offset < total; offset += c.perPage {
// Search the indicators
tqResponse, err := c.advancedSearch(query, token, offset)
if err != nil {
logrus.Errorf("Error while getting the indicators from TQ : %s", err)
continue
}
...
The rule is simple: if a variable is accessed from multiple goroutines and at least one of them is a write, explicit synchronization is needed.
This is true in your case: one of your goroutine writes the token variable (and also the err variable!), and another reads it, so you must synchronize access.
Since token is not a field of the threatq structure, putting the mutex that protects it would not be wise. Always put the mutex close to the data it ought to protect.
Some notes: as mentioned earlier, you also write and read the local err variable from multiple goroutines. You should not do this, instead create another local variable to hold the error from other goroutines (unless you want to "translfer" the error between goroutines, but this is not the case here).
See related questions:
Immutability of string and concurrency
Should we synchronize variable assignment in goroutine?
golang struct concurrent read and write without Lock is also running ok?
Yes, you could also try to run this test with -race flag enabled. Go's race detector will probably tell you that token is a shared variable across multiple goroutines. Thus, it must be protected with a Mutex or RWMutex.
In your case I think that RWMutex is more appropriate because there is one goroutine that changes (i.e. write) the state of token every 30 mins and another goroutine that reads its value.
If you don't protect the shared variable with a lock, the second goroutine might read an old value of token, that could be expired.