From 25cfb2087bd6d5c5c1b95f4d5a1a2b7dc4322e51 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Fri, 17 Dec 2021 16:19:00 +0100 Subject: [PATCH] cue/cuecontext: allow concurrent use This was currently only not allowed for imports. Really import indices should be unique per context, but even in that case, concurrency should be allowed. Fixes #1414 Signed-off-by: Marcel van Lohuizen Change-Id: I944357c7b68cd242d19b323a1380c751a7fe49a5 Signed-off-by: Marcel van Lohuizen Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/529629 Reviewed-by: Paul Jolly --- cue/cuecontext/cuecontext_test.go | 18 ++++++++++++++++++ internal/core/runtime/build.go | 2 +- internal/core/runtime/imports.go | 24 ++++++++++++++++++++---- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/cue/cuecontext/cuecontext_test.go b/cue/cuecontext/cuecontext_test.go index 2a780aa1ecb..aa5444a419c 100644 --- a/cue/cuecontext/cuecontext_test.go +++ b/cue/cuecontext/cuecontext_test.go @@ -54,3 +54,21 @@ func TestAPI(t *testing.T) { }) } } + +// TestConcurrency tests whether concurrent use of an index is allowed. +// This test only functions well with the --race flag. +func TestConcurrency(t *testing.T) { + c := New() + go func() { + c.CompileString(` + package foo + a: 1 + `) + }() + go func() { + c.CompileString(` + package bar + a: 2 + `) + }() +} diff --git a/internal/core/runtime/build.go b/internal/core/runtime/build.go index e3fb7ab1fe2..da5634b4a5b 100644 --- a/internal/core/runtime/build.go +++ b/internal/core/runtime/build.go @@ -135,7 +135,7 @@ func (x *Runtime) buildSpec(cfg *Config, b *build.Instance, spec *ast.ImportSpec return nil // TODO: check the builtin package exists here. } - if v := x.index.importsByBuild[pkg]; v != nil { + if v := x.getNodeFromInstance(pkg); v != nil { return pkg.Err } diff --git a/internal/core/runtime/imports.go b/internal/core/runtime/imports.go index 243db4fdd5c..403a8a051b3 100644 --- a/internal/core/runtime/imports.go +++ b/internal/core/runtime/imports.go @@ -58,13 +58,17 @@ var sharedIndex = newIndex() // // All instances belonging to the same package should share this index. type index struct { - // Change this to Instance at some point. - // From *structLit/*Vertex -> Instance + // lock is used to guard imports-related maps. + // TODO: makes these per cuecontext. + lock sync.RWMutex imports map[*adt.Vertex]*build.Instance importsByPath map[string]*adt.Vertex importsByBuild map[*build.Instance]*adt.Vertex - builtinPaths map[string]PackageFunc // Full path - builtinShort map[string]string // Commandline shorthand + + // These are initialized during Go package initialization time and do not + // need to be guarded. + builtinPaths map[string]PackageFunc // Full path + builtinShort map[string]string // Commandline shorthand typeCache sync.Map // map[reflect.Type]evaluated } @@ -86,6 +90,9 @@ func (x *index) shortBuiltinToPath(id string) string { } func (r *Runtime) AddInst(path string, key *adt.Vertex, p *build.Instance) { + r.index.lock.Lock() + defer r.index.lock.Unlock() + x := r.index if key == nil { panic("key must not be nil") @@ -98,14 +105,23 @@ func (r *Runtime) AddInst(path string, key *adt.Vertex, p *build.Instance) { } func (r *Runtime) GetInstanceFromNode(key *adt.Vertex) *build.Instance { + r.index.lock.RLock() + defer r.index.lock.RUnlock() + return r.index.imports[key] } func (r *Runtime) getNodeFromInstance(key *build.Instance) *adt.Vertex { + r.index.lock.RLock() + defer r.index.lock.RUnlock() + return r.index.importsByBuild[key] } func (r *Runtime) LoadImport(importPath string) (*adt.Vertex, errors.Error) { + r.index.lock.Lock() + defer r.index.lock.Unlock() + x := r.index key := x.importsByPath[importPath]