Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Fix data race in decodeStruct #101

Merged
merged 1 commit into from
Oct 17, 2017
Merged

Fix data race in decodeStruct #101

merged 1 commit into from
Oct 17, 2017

Conversation

magiconair
Copy link
Contributor

@magiconair magiconair commented Sep 26, 2017

Using *reflect.StructField as map key results in a data race. I honestly
do not understand why yet and can only reproduce this in the consul
codebase with high concurrency but this patch fixes it.

The same problem also exists in hashicorp/hcl.

hashicorp/hcl#213

Using *reflect.StructField as map key results in a data race. I honestly
do not understand why yet and can only reproduce this in the consul
codebase with high concurrency but this patch fixes it.

The same problem also exists in hashicorp/hcl.
magiconair added a commit to hashicorp/consul that referenced this pull request Sep 26, 2017
@mitchellh mitchellh merged commit 06020f8 into mitchellh:master Oct 17, 2017
@magiconair magiconair deleted the fix-decodeStruct-data-race branch October 17, 2017 18:33
@danielchatfield
Copy link

@magiconair Did you ever get to the bottom of how this data race can occur?

We've experienced this in production and I can't work out (barring bugs in the Go runtime) how this could cause a race. I've updated this dependency within our codebase but it is somewhat unsatisfying to not know why.

@magiconair
Copy link
Contributor Author

@danielchatfield no idea unfortunately.

@jackkleeman
Copy link

@magiconair do you have any repro code?

@magiconair
Copy link
Contributor Author

I think what I did was to run go test -parallel 128 -race

@jackkleeman
Copy link

Ok thanks!

@jackkleeman
Copy link

@magiconair were you able to reliably reproduce? We are not able to. In production at one time we had the race repeatedly, but now we can't reproduce there either. We can't figure out what might be changing from one day to the next.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants