Fix for multiple goroutines appending to slice, Solves #613 #639
Conversation
| @@ -262,6 +262,7 @@ type changeSliceArg struct { | |||
| localParent, remoteParent string | |||
| dirList []*dirList | |||
| changeListPtr *[]*Change | |||
| mtx *sync.Mutex | |||
There was a problem hiding this comment.
Could you make this instead a non-pointer value and drop the extra the indirection ie
mtx sync.MutexThere was a problem hiding this comment.
Also could we call it mu sync.Mutex?
There was a problem hiding this comment.
changing to mu is fine. I'll send an update for that.
The non-pointer I think will be an issue. Each go routine needs to be using the same mutex to effectively communicate. So if I pass the mutex by value, I don't think they will be. I did a quick test and -race was still showing the race condition.
The other option would be to pass a channel and have the workers return values via the channel and add them to the list using a single routine.
There was a problem hiding this comment.
Ahhh, gotcha. Aye aye, go for it. Thank you for investigating this.
There was a problem hiding this comment.
I meant to say, you can keep the pointer version of it.
|
Just posted a few comments otherwise LGTM! Thank you. |
|
No problem it was fun getting to play with the race detector. |
|
LGTM! Thank you @sselph for this fix. |
No description provided.