Struggling w/ use of a waitgroup

[Robert Solomon]

https://go.dev/play/p/gIVVLsiTqod

I'm trying to understand concurrency, so I modified a small routine I came across quite a while ago.  It's a grep command, but since I have its source, I am trying to understand its concurrency.

My problem is that when there are more than about 1800 files to be processed, the go routines all deadlock.

This code works fine as long as I have fewer than about 1800 files to process.

I don't understand why this happens.

Windows 10.

Tried w/ go1.17, go1.18 and go1.19

--Rob Solomon

[Jan Mercl]

On Sun, Oct 2, 2022 at 7:36 PM Robert Solomon <drro...@gmail.com> wrote:

> https://go.dev/play/p/gIVVLsiTqod

I believe wg.Add on line 125 is too late. I think it needs to be moved
before the go statement on line 108. Not tested.

[Matthew Zimmerman]

First reason I notice, if there's an error opening your file, wg.Done() is never called.  

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/3799e520-9a98-4117-b407-f6aea24995ccn%40googlegroups.com.

[rob]

When I do that, I get this error:

panic: sync: negative WaitGroup number

[Jan Mercl]

On Sun, Oct 2, 2022 at 8:52 PM rob <drro...@gmail.com> wrote:

> When I do that, I get this error:
>
> panic: sync: negative WaitGroup number

I tried to investigate, but the code does not build.

./main.go:99:11: undefined: globCommandLineFiles
./main.go:101:11: undefined: commandLineFiles

Can you please make it a self-contained, minimal program that
reproduces the failure?

[Andrew Harris]

I think Matthew is correct about the immediate source of the deadlock - because the defer is placed too late in the body of grepFile(), the deferred decrement of the waitGroup isn't run on an os.Open() error.

I had the same impression as Jan, I think there is a concern here: the condition for closing grepChan could be, when all files have been sent into the channel. Currently, it's when all work is done. The difference is that one pattern is to increment/decrement waitGroup per unit of work, but another one is to increment/decrement just workers. For the latter, the worker decrements when it infers there is no more work to be done, i.e. it reads that grepChan has closed. I think changing the pattern would avoid the deadlock because, then, there's no reason to interact with the waitGroup per file.

[rob]

Looks like you're right.

I changed the order of the defer statement and now I'm not getting that error.

Interesting that I never saw any file errors.

Thanks

--rob solomon

[Robert Solomon]

I don't know how to change that. 

I had an issue w/ the main routine ending before some of the worker routines were done so I saw incomplete results.

How do I tweak my logic so that I don't need a WaitGroup? 

I forgot about my platform specific code.  Here is the rest of that, that is in a file to be compiled on windows.

package main

import (
    "fmt"
    "os"
    "path/filepath"
    "strings"
) // Glob is case sensitive.  I want case insensitive.

const estimatedNumberOfFiles = 100

func globCommandLineFiles(patterns []string) []string {
    matchingNames := make([]string, 0, estimatedNumberOfFiles)
    for _, pattern := range patterns {
        matches, err := filepath.Glob(pattern) // Glob returns names of all files matching the case-sensitive pattern.
        if err != nil {
            fmt.Fprintln(os.Stderr, " Error from filepath.Glob is", err)
            os.Exit(1)
        } else if matches != nil { // At least one match
            matchingNames = append(matchingNames, matches...)
        }
    }
    return matchingNames
}

func commandLineFiles(patterns []string) []string {
    workingDirname, err := os.Getwd()
    if err != nil {
        fmt.Fprintln(os.Stderr, "from commandlinefiles:", err)
        return nil
    }
    dirEntries, e := os.ReadDir(workingDirname) // became available as of Go 1.16
    if e != nil {
        return nil
    }

    matchingNames := make([]string, 0, len(dirEntries))

    for _, pattern := range patterns { // outer loop to test against multiple patterns.
        for _, d := range dirEntries { // inner loop to test each pattern against the filenames.
            if d.IsDir() {
                continue // skip a subdirectory name
            }
            boolean, er := filepath.Match(pattern, strings.ToLower(d.Name()))
            if er != nil {
                fmt.Fprintln(os.Stderr, er)
                continue
            }
            if boolean {
                matchingNames = append(matchingNames, d.Name())
            }
        }
    }
    return matchingNames
}