go/hcl
2
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix data race in decodeStruct

Browse Source 
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 mitchellh/mapstructure.
This commit is contained in:
Frank Schroeder 2017-09-26 12:36:24 +02:00
parent 68e816d1c7
commit 42206063bf
No known key found for this signature in database
GPG Key ID: 4D65C6EAEC87DECD
1 changed files with 18 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
decoder.go
View File

@ -573,7 +573,11 @@ func (d *decoder) decodeStruct(name string, node ast.Node, result reflect.Value)
// Compile the list of all the fields that we're going to be decoding // Compile the list of all the fields that we're going to be decoding
// from all the structs. // from all the structs.
fields := make(map[*reflect.StructField]reflect.Value) type field struct {
field reflect.StructField
val reflect.Value
}
fields := []field{}
for len(structs) > 0 { for len(structs) > 0 {
structVal := structs[0] structVal := structs[0]
structs = structs[1:] structs = structs[1:]
@ -616,7 +620,7 @@ func (d *decoder) decodeStruct(name string, node ast.Node, result reflect.Value)
} }
// Normal struct field, store it away // Normal struct field, store it away
fields[&fieldType] = structVal.Field(i) fields = append(fields, field{fieldType, structVal.Field(i)})
} }
} }
@ -624,26 +628,27 @@ func (d *decoder) decodeStruct(name string, node ast.Node, result reflect.Value)
decodedFields := make([]string, 0, len(fields)) decodedFields := make([]string, 0, len(fields))
decodedFieldsVal := make([]reflect.Value, 0) decodedFieldsVal := make([]reflect.Value, 0)
unusedKeysVal := make([]reflect.Value, 0) unusedKeysVal := make([]reflect.Value, 0)
for fieldType, field := range fields { for _, f := range fields {
if !field.IsValid() { field, fieldValue := f.field, f.val
if !fieldValue.IsValid() {
// This should never happen // This should never happen
panic("field is not valid") panic("field is not valid")
} }
// If we can't set the field, then it is unexported or something, // If we can't set the field, then it is unexported or something,
// and we just continue onwards. // and we just continue onwards.
if !field.CanSet() { if !fieldValue.CanSet() {
continue continue
} }
fieldName := fieldType.Name fieldName := field.Name
tagValue := fieldType.Tag.Get(tagName) tagValue := field.Tag.Get(tagName)
tagParts := strings.SplitN(tagValue, ",", 2) tagParts := strings.SplitN(tagValue, ",", 2)
if len(tagParts) >= 2 { if len(tagParts) >= 2 {
switch tagParts[1] { switch tagParts[1] {
case "decodedFields": case "decodedFields":
decodedFieldsVal = append(decodedFieldsVal, field) decodedFieldsVal = append(decodedFieldsVal, fieldValue)
continue continue
case "key": case "key":
if item == nil { if item == nil {
@ -654,10 +659,10 @@ func (d *decoder) decodeStruct(name string, node ast.Node, result reflect.Value)
} }
} }
field.SetString(item.Keys[0].Token.Value().(string)) fieldValue.SetString(item.Keys[0].Token.Value().(string))
continue continue
case "unusedKeys": case "unusedKeys":
unusedKeysVal = append(unusedKeysVal, field) unusedKeysVal = append(unusedKeysVal, fieldValue)
continue continue
} }
} }
@ -684,7 +689,7 @@ func (d *decoder) decodeStruct(name string, node ast.Node, result reflect.Value)
// because we actually want the value. // because we actually want the value.
fieldName = fmt.Sprintf("%s.%s", name, fieldName) fieldName = fmt.Sprintf("%s.%s", name, fieldName)
if len(prefixMatches.Items) > 0 { if len(prefixMatches.Items) > 0 {
if err := d.decode(fieldName, prefixMatches, field); err != nil { if err := d.decode(fieldName, prefixMatches, fieldValue); err != nil {
return err return err
} }
} }
@ -694,12 +699,12 @@ func (d *decoder) decodeStruct(name string, node ast.Node, result reflect.Value)
decodeNode = &ast.ObjectList{Items: ot.List.Items} decodeNode = &ast.ObjectList{Items: ot.List.Items}
} }
if err := d.decode(fieldName, decodeNode, field); err != nil { if err := d.decode(fieldName, decodeNode, fieldValue); err != nil {
return err return err
} }
} }
decodedFields = append(decodedFields, fieldType.Name) decodedFields = append(decodedFields, field.Name)
} }
if len(decodedFieldsVal) > 0 { if len(decodedFieldsVal) > 0 {