WaitGroup is reused before previous Wait has returned

Question

So im getting further into using golang and looking more into the concurrency it offers. I decided to try to use go routines to implement permutations of strings in a phone number.

I am running into issues using sync.WaitGroup to coordinate the go routines that i have been using. the specific error being:

WaitGroup is reused before previous Wait has returned

the code being:

main.go

package main

import (
    "fmt"
    "sync"

    "github.com/sbiscigl/phonenumberperm/intstack"
    "github.com/sbiscigl/phonenumberperm/permutations"
)

var wg sync.WaitGroup

func main() {
    num := []int{2, 7, 1, 4, 5, 5, 2}    
    stack := intstack.New(num)
    permutationChannel := make(chan string)
    wg.Add(1)
    go permutations.ThreadSafeCalcWords(stack, "", permutationChannel, &wg)
    wg.Wait()
    /*Then consume, but not implimented*/
}

permutations/perm.go

package permutations

import (
    "fmt"
    "sync"

    "github.com/sbiscigl/phonenumberperm/intstack"
)

var letterMap = map[int][]string{
    2: []string{"a", "b", "c"},
    3: []string{"d", "e", "f"},
    4: []string{"g", "h", "i"},
    5: []string{"j", "k", "l"},
    6: []string{"m", "n", "o"},
    7: []string{"p", "q", "r", "s"},
    8: []string{"t", "u", "v"},
    9: []string{"w", "x", "y", "z"},
}

func ThreadSafeCalcWords(s intstack.IntStack, word string, ch chan<- string,
    wg *sync.WaitGroup) {
    if s.IsEmpty() {
        ch <- fmt.Sprint(word)
        wg.Done()
    } else {
        /*Check to see if the values are 1 or zero as they*/
        /*have no letters associated with them*/
        if s.Peek() == 1 || s.Peek() == 0 {
            wg.Done()
            s.Pop()
            wg.Add(1)
            go ThreadSafeCalcWords(s, word, ch, wg)
        } else {
            wg.Done()
            for _, letter := range letterMap[s.Pop()] {
                wg.Add(1)
                go ThreadSafeCalcWords(s, word+letter, ch, wg)
            }
        }
    }
}

intstack/intstack.go

package intstack

import "fmt"

const (
    maxSize = 100
)

/*IntStack implimentaiton of a stack for integers*/
type IntStack struct {
    valueList []int
    maxSize   int
}

/*New returns bew instace of IntStack*/
func New(nums []int) IntStack {
    return IntStack{
        valueList: nums,
        maxSize:   maxSize,
    }
}

/*Pop pops the top value off the stack*/
func (s *IntStack) Pop() int {
    var val int
    if !s.IsEmpty() {
        val = s.valueList[0]
        s.valueList = s.valueList[1:]
    } else {
        fmt.Println("stack is empty")
    }
    return val
}

/*Peek returns top value*/
func (s IntStack) Peek() int {
    return s.valueList[0]
}

/*IsEmpty checks if the stack is empty*/
func (s IntStack) IsEmpty() bool {
    if len(s.valueList) > 0 {
        return false
    }
    return true
}

/*Print prints out the contents of the stack*/
func (s IntStack) Print() {
    for _, element := range s.valueList {
        fmt.Print(element)
    }
    fmt.Print("\n")
}

So after research that line is called during wg.Wait() or rathe wait groups wait function. I tried to replicate with a smaller program, but could not. My hypothesis is that once it reaches the the Wait() after it calls the the go routine, that we can no longer edit the wait group, but that sounds wrong. any insight into why this would be happening would be helpful

repo for refrence can be found at: https://github.com/sbiscigl/phonenumberperm

Answer 1

In your recursive cases in ThreadSafeCalcWords, you're calling wg.Done before calling wg.Add. That means that the wg can drop down to 0 (which will trigger the Wait to complete) before you actually finish all the work. Calling Add again while the Wait is still in the process of resolving is what triggers the error, but more importantly, it probably just plain isn't what you want.

Change the order of operations so that you always Add any new work before doing Done on the existing work, and the Wait won't trigger prematurely. The simplest way to accomplish this would probably be a single call to wg.Done() at the bottom of the function, or a single defer at the top, and removing all the others.