godu icon indicating copy to clipboard operation
godu copied to clipboard

Possible race codition

Open dundee opened this issue 5 years ago • 2 comments

WaitGroup.Add method should be called only from the main goroutine.

// Note that calls with a positive delta that occur when the counter is zero
// must happen before a Wait. Calls with a negative delta, or calls with a
// positive delta that start when the counter is greater than zero, may happen
// at any time.

This is not true in walkSubFolderConcurrently since it's called recursivelly from the nested goroutines.

dundee avatar Mar 25 '21 23:03 dundee

Thanks for creating the issue @dundee. Do you suggest that

calls with a positive delta that occur when the counter is zero

might happen after we call the WaitGroup.Wait()? Could you please expand the issue description with a scenario when a race condition occurs?

viktomas avatar Apr 03 '21 08:04 viktomas

Yes, it's not very likely, but it can happen.

There can happen that first Add - https://github.com/viktomas/godu/blob/master/files/file_walker.go#L96 and following Done - https://github.com/viktomas/godu/blob/master/files/file_walker.go#L104 - will process very quickly, so that any other Add calls are not processed yet.

There can be for example one directory (which will do the first Add and Done), then many many files (which will slow the main goroutine) and then second directory which will call Add too late.

Therefore Add calls should be done only in the main goroutine.

// Typically this means the calls to Add should execute before the statement // creating the goroutine or other event to be waited for.

I have crossed this problem in gdu where I was using very similar approach to yours. I have rewritten it to use custom wait group - https://github.com/dundee/gdu/commit/c30b51305b56e5fc6d47500d396a5b23622d6f26 - which is a bit slower but uses mutex locking.

dundee avatar Apr 03 '21 22:04 dundee