Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix data race in LoadStandardTags #63

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion v3/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ go 1.12
// replace github.com/dsoprea/go-utility/v2 => ../../go-utility/v2

require (
github.com/dsoprea/go-logging v0.0.0-20200517223158-a10564966e9d
github.com/dsoprea/go-logging v0.0.0-20200710184922-b02d349568dd
github.com/dsoprea/go-utility/v2 v2.0.0-20200717064901-2fccff4aa15e
github.com/golang/geo v0.0.0-20200319012246-673a6f80352d
github.com/jessevdk/go-flags v1.4.0
Expand Down
2 changes: 2 additions & 0 deletions v3/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ github.com/dsoprea/go-logging v0.0.0-20200502201358-170ff607885f h1:FonKAuW3PmNt
github.com/dsoprea/go-logging v0.0.0-20200502201358-170ff607885f/go.mod h1:7I+3Pe2o/YSU88W0hWlm9S22W7XI1JFNJ86U0zPKMf8=
github.com/dsoprea/go-logging v0.0.0-20200517223158-a10564966e9d h1:F/7L5wr/fP/SKeO5HuMlNEX9Ipyx2MbH2rV9G4zJRpk=
github.com/dsoprea/go-logging v0.0.0-20200517223158-a10564966e9d/go.mod h1:7I+3Pe2o/YSU88W0hWlm9S22W7XI1JFNJ86U0zPKMf8=
github.com/dsoprea/go-logging v0.0.0-20200710184922-b02d349568dd h1:l+vLbuxptsC6VQyQsfD7NnEC8BZuFpz45PgY+pH8YTg=
github.com/dsoprea/go-logging v0.0.0-20200710184922-b02d349568dd/go.mod h1:7I+3Pe2o/YSU88W0hWlm9S22W7XI1JFNJ86U0zPKMf8=
github.com/dsoprea/go-utility v0.0.0-20200711062821-fab8125e9bdf h1:/w4QxepU4AHh3AuO6/g8y/YIIHH5+aKP3Bj8sg5cqhU=
github.com/dsoprea/go-utility v0.0.0-20200711062821-fab8125e9bdf/go.mod h1:95+K3z2L0mqsVYd6yveIv1lmtT3tcQQ3dVakPySffW8=
github.com/dsoprea/go-utility/v2 v0.0.0-20200512094054-1abbbc781176 h1:CfXezFYb2STGOd1+n1HshvE191zVx+QX3A1nML5xxME=
Expand Down
115 changes: 59 additions & 56 deletions v3/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"fmt"
"sync"

"github.com/dsoprea/go-logging"
log "github.com/dsoprea/go-logging"
"gopkg.in/yaml.v2"

"github.com/dsoprea/go-exif/v3/common"
exifcommon "github.com/dsoprea/go-exif/v3/common"
)

const (
Expand Down Expand Up @@ -176,6 +176,7 @@ func (it *IndexedTag) DoesSupportType(tagType exifcommon.TagTypePrimitive) bool

// TagIndex is a tag-lookup facility.
type TagIndex struct {
tagsInit sync.Once
tagsByIfd map[string]map[uint16]*IndexedTag
tagsByIfdR map[string]map[string]*IndexedTag

Expand Down Expand Up @@ -253,10 +254,8 @@ func (ti *TagIndex) getOne(ifdPath string, id uint16) (it *IndexedTag, err error
}
}()

if len(ti.tagsByIfd) == 0 {
err := LoadStandardTags(ti)
log.PanicIf(err)
}
err = LoadStandardTags(ti)
bep marked this conversation as resolved.
Show resolved Hide resolved
log.PanicIf(err)

ti.mutex.Lock()
defer ti.mutex.Unlock()
Expand Down Expand Up @@ -384,10 +383,8 @@ func (ti *TagIndex) GetWithName(ii *exifcommon.IfdIdentity, name string) (it *In
}
}()

if len(ti.tagsByIfdR) == 0 {
err := LoadStandardTags(ti)
log.PanicIf(err)
}
err = LoadStandardTags(ti)
log.PanicIf(err)

ifdPath := ii.UnindexedString()

Expand All @@ -408,68 +405,74 @@ func LoadStandardTags(ti *TagIndex) (err error) {
}
}()

// Read static data.
ti.tagsInit.Do(func() {
if len(ti.tagsByIfd) != 0 {
return
}
// Read static data.

encodedIfds := make(map[string][]encodedTag)
encodedIfds := make(map[string][]encodedTag)

err = yaml.Unmarshal([]byte(tagsYaml), encodedIfds)
log.PanicIf(err)
err = yaml.Unmarshal([]byte(tagsYaml), encodedIfds)
log.PanicIf(err)

// Load structure.
// Load structure.

count := 0
for ifdPath, tags := range encodedIfds {
for _, tagInfo := range tags {
tagId := uint16(tagInfo.Id)
tagName := tagInfo.Name
tagTypeName := tagInfo.TypeName
tagTypeNames := tagInfo.TypeNames

if tagTypeNames == nil {
if tagTypeName == "" {
log.Panicf("no tag-types were given when registering standard tag [%s] (0x%04x) [%s]", ifdPath, tagId, tagName)
}

tagTypeNames = []string{
tagTypeName,
}
} else if tagTypeName != "" {
log.Panicf("both 'type_names' and 'type_name' were given when registering standard tag [%s] (0x%04x) [%s]", ifdPath, tagId, tagName)
}

count := 0
for ifdPath, tags := range encodedIfds {
for _, tagInfo := range tags {
tagId := uint16(tagInfo.Id)
tagName := tagInfo.Name
tagTypeName := tagInfo.TypeName
tagTypeNames := tagInfo.TypeNames
tagTypes := make([]exifcommon.TagTypePrimitive, 0)
for _, tagTypeName := range tagTypeNames {

if tagTypeNames == nil {
if tagTypeName == "" {
log.Panicf("no tag-types were given when registering standard tag [%s] (0x%04x) [%s]", ifdPath, tagId, tagName)
}
// TODO(dustin): Discard unsupported types. This helps us with non-standard types that have actually been found in real data, that we ignore for right now. e.g. SSHORT, FLOAT, DOUBLE
tagTypeId, found := exifcommon.GetTypeByName(tagTypeName)
if found == false {
tagsLogger.Warningf(nil, "Type [%s] for tag [%s] being loaded is not valid and is being ignored.", tagTypeName, tagName)
continue
}

tagTypeNames = []string{
tagTypeName,
tagTypes = append(tagTypes, tagTypeId)
}
} else if tagTypeName != "" {
log.Panicf("both 'type_names' and 'type_name' were given when registering standard tag [%s] (0x%04x) [%s]", ifdPath, tagId, tagName)
}

tagTypes := make([]exifcommon.TagTypePrimitive, 0)
for _, tagTypeName := range tagTypeNames {

// TODO(dustin): Discard unsupported types. This helps us with non-standard types that have actually been found in real data, that we ignore for right now. e.g. SSHORT, FLOAT, DOUBLE
tagTypeId, found := exifcommon.GetTypeByName(tagTypeName)
if found == false {
tagsLogger.Warningf(nil, "Type [%s] for tag [%s] being loaded is not valid and is being ignored.", tagTypeName, tagName)
if len(tagTypes) == 0 {
tagsLogger.Warningf(nil, "Tag [%s] (0x%04x) [%s] being loaded does not have any supported types and will not be registered.", ifdPath, tagId, tagName)
continue
}

tagTypes = append(tagTypes, tagTypeId)
}
it := &IndexedTag{
IfdPath: ifdPath,
Id: tagId,
Name: tagName,
SupportedTypes: tagTypes,
}

if len(tagTypes) == 0 {
tagsLogger.Warningf(nil, "Tag [%s] (0x%04x) [%s] being loaded does not have any supported types and will not be registered.", ifdPath, tagId, tagName)
continue
}
err = ti.Add(it)
log.PanicIf(err)

it := &IndexedTag{
IfdPath: ifdPath,
Id: tagId,
Name: tagName,
SupportedTypes: tagTypes,
count++
}

err = ti.Add(it)
log.PanicIf(err)

count++
}
}

tagsLogger.Debugf(nil, "(%d) tags loaded.", count)
tagsLogger.Debugf(nil, "(%d) tags loaded.", count)

})

return nil
}