Golang multiple goroutine sharing same variable by reference

Issue

I am trying to run multiple goroutines that modify the same variable passed by reference.

But I am sure the way I have implemented this is functionally incorrect. Even though it seems to be working in my tests, I have a feeling this pattern would end the parent function when the first goroutine completes if the second goroutine takes considerably longer to run than the first one.

I would like your input/suggestions/advice.

package auth

import (
    "regexp"

    zxcvbn "github.com/nbutton23/zxcvbn-go"
    "golang.org/x/net/context"
)

type AuthService struct{}

func NewAuthService() *AuthService {
    return &AuthService{}
}

func (this *AuthService) ValidateCredentials(ctx context.Context, req *ValidateCredentialsRequest) (*ValidateCredentialsResponse, error) {
    c := make(chan *ValidateCredentialsResponse)

    go validatePassword(req.GetPassword(), c)
    go validateUsername(req.GetUsername(), c)

    c <- &ValidateCredentialsResponse{IsValid: true}

    return <-c, nil
}

func validateUsername(email string, c chan *ValidateCredentialsResponse) {
    for {
        res := <-c

        if email == "" {
            res.IsValid = false
            res.Username = "Please provide your email address."
        } else if len(email) > 128 {
            res.IsValid = false
            res.Username = "Email address can not exceed 128 characters."
        } else if !regexp.MustCompile(`.+@.+`).MatchString(email) {
            res.IsValid = false
            res.Username = "Please enter a valid email address."
        }

        c <- res
    }
}

func validatePassword(password string, c chan *ValidateCredentialsResponse) {
    for {
        res := <-c

        if password == "" {
            res.IsValid = false
            res.Password = "Please provide your password."
        } else {
            quality := zxcvbn.PasswordStrength(password, []string{})
            if quality.Score < 3 {
                res.IsValid = false
                res.Password = "Your password is weak."
            }
        }

        c <- res
    }
}

Solution

Are you sure you need goroutines to perform simple validations?
Anyway the code you have written uses goroutines, but they are not running in parallel.

What’s going on in your code:
you create non-buffered channel and put CredentialResponse variable into it.
Then one goroutine (any of two) reads variable from channel, performs some actions, and puts variable back to the channel.
While first goroutine was doing some actions, second one was just waiting for a value from a channel.

So your code uses goroutines, but it can hardly be called parallel.

You may want to use goroutines if you need some heavy operations to validate data: io ops, or CPU, but in case of CPU you need specify GOMAXPROCS>1 to get some performance gain.

If I’d wanted to use goroutines for validation, I’d have written smth like it:

func validateCredentials(req *ValidateCredentialsRequest){
    ch := make(chan bool, 2)
    go func(name string){
    // ... validation code
        ch <- true // or false

    }(req.GetUsername())

    go func(pwd string){
    // ... validation code
        ch <- true // or false
    }(req.GetPassword())

    valid := true
    for i := 0; i < 2; i++ {
        v := <- result
        valid = valid && v
    }

    // ...
}

Answered By – n-canter

Answer Checked By – Candace Johnson (GoLangFix Volunteer)

Leave a Reply

Your email address will not be published.