Skip to content

Commit ca2400f

Browse files
committed
Fix race condition while preserving ParentProxy relationships
The previous fix eliminated the race condition by removing ParentProxy assignment for cached schemas, but broke the parent relationships that are critical for schema navigation. This fix creates shallow copies of schemas when returning from cache, ensuring each SchemaProxy gets its own Schema instance with the correct ParentProxy set. This eliminates race conditions while maintaining proper parent relationships and preserving memory efficiency through shallow copying. - Cache schemas without ParentProxy assignment to avoid races - Return shallow copies with correct ParentProxy for each caller - Add comprehensive test for ParentProxy preservation in cached schemas - All tests pass with race detection enabled
1 parent 8e5fb02 commit ca2400f

File tree

2 files changed

+63
-4
lines changed

2 files changed

+63
-4
lines changed

datamodel/high/base/schema_proxy.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,11 @@ func (sp *SchemaProxy) Schema() *Schema {
116116
loc := fmt.Sprintf("%s:%d:%d", idx.GetSpecAbsolutePath(), sp.schema.GetValueNode().Line, sp.schema.GetValueNode().Column)
117117
if seen, ok := idx.GetHighCache().Load(loc); ok {
118118
idx.HighCacheHit()
119-
return seen.(*Schema)
119+
// Create a copy of the cached schema with the correct ParentProxy
120+
cachedSchema := seen.(*Schema)
121+
schemaCopy := *cachedSchema // shallow copy
122+
schemaCopy.ParentProxy = sp
123+
return &schemaCopy
120124
} else {
121125
idx.HighCacheMiss()
122126
}
@@ -144,9 +148,11 @@ func (sp *SchemaProxy) Schema() *Schema {
144148
}
145149
}
146150

147-
sch.ParentProxy = sp
148-
sp.rendered = sch
149-
return sch
151+
// Create a copy of the schema to avoid race conditions when setting ParentProxy
152+
schemaCopy := *sch // shallow copy
153+
schemaCopy.ParentProxy = sp
154+
sp.rendered = &schemaCopy
155+
return &schemaCopy
150156
}
151157

152158
// IsReference returns true if the SchemaProxy is a reference to another Schema.

datamodel/high/base/schema_proxy_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,3 +519,56 @@ func TestSchemaProxy_ConcurrentCacheAccess(t *testing.T) {
519519
}
520520
wg.Wait()
521521
}
522+
523+
func TestSchemaProxy_ParentProxyPreservedForCachedSchemas(t *testing.T) {
524+
// Create schema that will be cached
525+
const ymlComponents = `components:
526+
schemas:
527+
TestSchema:
528+
type: object
529+
properties:
530+
name:
531+
type: string`
532+
533+
var idxNode yaml.Node
534+
err := yaml.Unmarshal([]byte(ymlComponents), &idxNode)
535+
assert.NoError(t, err)
536+
idx := index.NewSpecIndexWithConfig(&idxNode, index.CreateOpenAPIIndexConfig())
537+
538+
// Create first proxy and get schema (this will cache it)
539+
const ymlSchema = `$ref: '#/components/schemas/TestSchema'`
540+
var node1 yaml.Node
541+
yaml.Unmarshal([]byte(ymlSchema), &node1)
542+
543+
lowProxy1 := new(lowbase.SchemaProxy)
544+
lowProxy1.Build(context.Background(), nil, node1.Content[0], idx)
545+
546+
proxy1 := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{
547+
Value: lowProxy1, ValueNode: node1.Content[0],
548+
})
549+
550+
// Get schema from first proxy (this should cache it)
551+
schema1 := proxy1.Schema()
552+
assert.NotNil(t, schema1)
553+
assert.Equal(t, proxy1, schema1.ParentProxy, "First schema should have correct ParentProxy")
554+
555+
// Create second proxy for same schema reference
556+
var node2 yaml.Node
557+
yaml.Unmarshal([]byte(ymlSchema), &node2)
558+
559+
lowProxy2 := new(lowbase.SchemaProxy)
560+
lowProxy2.Build(context.Background(), nil, node2.Content[0], idx)
561+
562+
proxy2 := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{
563+
Value: lowProxy2, ValueNode: node2.Content[0],
564+
})
565+
566+
// Get schema from second proxy (this should return cached schema)
567+
schema2 := proxy2.Schema()
568+
assert.NotNil(t, schema2)
569+
570+
// CRITICAL TEST: Each proxy should have its own parent relationship
571+
// Currently fails due to the race condition fix
572+
assert.Equal(t, proxy2, schema2.ParentProxy, "Second schema should have its own ParentProxy, not the first proxy's")
573+
assert.NotEqual(t, schema1.ParentProxy, schema2.ParentProxy, "Different proxies should have different parent relationships")
574+
}

0 commit comments

Comments
 (0)