Do I need a mutex when replacing the value of a string in different goroutines?

Question

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
            }

...

Of course. But when in doubt: Run it under the race detector. — Volker, May 22 '19 at 10:18

icza · Accepted Answer · 2019-05-22T10:02:58.473

1

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?

Reading values from a different thread

Why does this code cause data race?

edited May 22 '19 at 10:02

answered May 22 '19 at 09:57

icza

389,944
63
907
827

thanks a lot. You're right, the token is not a field of my structure, so do I need to use a simple "standalone" mutex inside the code directly ? – fallais May 22 '19 at 10:05
@Elwyn Put the mutex next to the `token` variable. If you have this pattern used in multiple places of your app, consider making a struct type wrapping the string field and the mutex protecting it, and you may even add methods to ensure safe concurrent reads / writes. For a nice example, see [When do you embed mutex in struct in Go?](https://stackoverflow.com/questions/44949467/when-do-you-embed-mutex-in-struct-in-go/44950096#44950096) – icza May 22 '19 at 10:06
this is where I am getting lost, how a simple mutex variable, next to the `token, err := c.authenticate(c.clientID, c.email, c.password)` is protecting it ? I perfectly see the power of mutex when it is nested into a struct and protecting it, but in this case... I am lost. – fallais May 22 '19 at 10:08
@Elwyn Putting it next to `token` does not protect the `token` variable. Neither does wrapping them in a struct. _Using_ the mutex properly is what protects the variable. You should put it next to `token` just so you know the _purpose_ of that mutex is to protect the `token` variable. – icza May 22 '19 at 10:16

score 0 · Answer 2 · answered May 22 '19 at 09:56

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.

Do I need a mutex when replacing the value of a string in different goroutines?

2 Answers2