From 61d3a4d6ea93e865738bf949657ff0ffcc8a7f97 Mon Sep 17 00:00:00 2001 From: Andy Bursavich Date: Thu, 3 Dec 2020 19:28:38 -0800 Subject: [PATCH] Fix schema initialization paths (#3825) * Fix schema initialization paths The initialized channel was only closed if the schema's cacheStore did not contain the embeddedCacheKey and there were no errors parsing relations. If the key existed or an error occurred, it would not be closed. This could leave other goroutines waiting for synchronization that will never occur. Additionally, the other code paths that wait for initialization to complete did not return the possible error. * Unnest common schema initialization This makes the common code path less deeply nested and the flow control easier to follow. --- schema/schema.go | 57 ++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/schema/schema.go b/schema/schema.go index da4be305..8d9368da 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -92,7 +92,7 @@ func Parse(dest interface{}, cacheStore *sync.Map, namer Namer) (*Schema, error) if v, ok := cacheStore.Load(modelType); ok { s := v.(*Schema) <-s.initialized - return s, nil + return s, s.err } modelValue := reflect.New(modelType) @@ -223,37 +223,38 @@ func Parse(dest interface{}, cacheStore *sync.Map, namer Namer) (*Schema, error) } } - if s, loaded := cacheStore.LoadOrStore(modelType, schema); !loaded { - if _, embedded := schema.cacheStore.Load(embeddedCacheKey); !embedded { - for _, field := range schema.Fields { - if field.DataType == "" && (field.Creatable || field.Updatable || field.Readable) { - if schema.parseRelation(field); schema.err != nil { - return schema, schema.err - } - } + if v, loaded := cacheStore.LoadOrStore(modelType, schema); loaded { + s := v.(*Schema) + <-s.initialized + return s, s.err + } - fieldValue := reflect.New(field.IndirectFieldType) - if fc, ok := fieldValue.Interface().(CreateClausesInterface); ok { - field.Schema.CreateClauses = append(field.Schema.CreateClauses, fc.CreateClauses(field)...) - } - - if fc, ok := fieldValue.Interface().(QueryClausesInterface); ok { - field.Schema.QueryClauses = append(field.Schema.QueryClauses, fc.QueryClauses(field)...) - } - - if fc, ok := fieldValue.Interface().(UpdateClausesInterface); ok { - field.Schema.UpdateClauses = append(field.Schema.UpdateClauses, fc.UpdateClauses(field)...) - } - - if fc, ok := fieldValue.Interface().(DeleteClausesInterface); ok { - field.Schema.DeleteClauses = append(field.Schema.DeleteClauses, fc.DeleteClauses(field)...) + defer close(schema.initialized) + if _, embedded := schema.cacheStore.Load(embeddedCacheKey); !embedded { + for _, field := range schema.Fields { + if field.DataType == "" && (field.Creatable || field.Updatable || field.Readable) { + if schema.parseRelation(field); schema.err != nil { + return schema, schema.err } } - close(schema.initialized) + + fieldValue := reflect.New(field.IndirectFieldType) + if fc, ok := fieldValue.Interface().(CreateClausesInterface); ok { + field.Schema.CreateClauses = append(field.Schema.CreateClauses, fc.CreateClauses(field)...) + } + + if fc, ok := fieldValue.Interface().(QueryClausesInterface); ok { + field.Schema.QueryClauses = append(field.Schema.QueryClauses, fc.QueryClauses(field)...) + } + + if fc, ok := fieldValue.Interface().(UpdateClausesInterface); ok { + field.Schema.UpdateClauses = append(field.Schema.UpdateClauses, fc.UpdateClauses(field)...) + } + + if fc, ok := fieldValue.Interface().(DeleteClausesInterface); ok { + field.Schema.DeleteClauses = append(field.Schema.DeleteClauses, fc.DeleteClauses(field)...) + } } - } else { - <-s.(*Schema).initialized - return s.(*Schema), nil } return schema, schema.err