diff --git a/bsup/loader/loader_test.go b/bsup/loader/loader_test.go index 9a4f9e0d7..896b0c731 100644 --- a/bsup/loader/loader_test.go +++ b/bsup/loader/loader_test.go @@ -31,6 +31,12 @@ type port=uint16 ` func newFrameLoader(t *testing.T, sctx *super.Context, s string) *loader.FrameLoader { + l, err := loader.NewFrameLoader(sctx, newFrame(t, sctx, s)) + require.NoError(t, err) + return l +} + +func newFrame(t *testing.T, sctx *super.Context, s string) *bsup.ColFrame { var buf bytes.Buffer w := bsup.NewColumnWriter(sio.NopCloser(&buf)) r := supio.NewReader(sctx, strings.NewReader(s)) @@ -47,9 +53,7 @@ func newFrameLoader(t *testing.T, sctx *super.Context, s string) *loader.FrameLo require.NoError(t, w.Close()) frame, err := bsup.NewSeekable(sctx, bytes.NewReader(buf.Bytes())).Next() require.NoError(t, err) - l, err := loader.NewFrameLoader(sctx, frame.(*bsup.ColFrame)) - require.NoError(t, err) - return l + return frame.(*bsup.ColFrame) } func load(l *loader.FrameLoader, projection field.Projection) string { @@ -124,6 +128,23 @@ func TestLoaderProjections(t *testing.T) { wg.Wait() } +func TestLoaderNamedConcurrentProjections(t *testing.T) { + // Concurrent projections that change a named type's inner type race to + // bind its name in a context where it isn't bound yet. + frame := newFrame(t, super.NewContext(), "type foo={a:int64,b:int64}\n{a:1,b:2}::foo\n") + for range 1000 { + l, err := loader.NewFrameLoader(super.NewContext(), frame) + require.NoError(t, err) + var wg sync.WaitGroup + for path, want := range map[string]string{"a": "{a:1}", "b": "{b:2}"} { + wg.Go(func() { + assert.Contains(t, load(l, projection(path)), want) + }) + } + wg.Wait() + } +} + func TestLoaderDynamic(t *testing.T) { // Values of different types make a dynamic root, whose values are // loaded together or separately. diff --git a/bsup/loader/named.go b/bsup/loader/named.go index 403ad1dff..f1eb7fb2d 100644 --- a/bsup/loader/named.go +++ b/bsup/loader/named.go @@ -17,15 +17,8 @@ func (n *Named) project(loader *FrameLoader, projection field.Projection) vector vec := n.Values.project(loader, projection) // Try to preserve the named type if possible but if the projection changes // the underlying type, then just return the inner vector. - inner := vec.Type() - named := loader.sctx.LookupByName(n.Name) - if named == nil { - var err error - named, err = loader.sctx.LookupTypeNamed(n.Name, vec.Type()) - if err != nil { - panic(err) - } - } else if named.Type != inner { + named, err := loader.sctx.LookupTypeNamed(n.Name, vec.Type()) + if err != nil { return vec } return vector.NewNamed(named, vec) diff --git a/context.go b/context.go index c652d5ca4..7a55bd3a1 100644 --- a/context.go +++ b/context.go @@ -375,25 +375,14 @@ func (c *Context) LookupTypeNamed(name string, inner Type) (*TypeNamed, error) { if !utf8.ValidString(name) { return nil, fmt.Errorf("bad type name %q: invalid UTF-8", name) } - if err := CheckTypeName(name); err != nil { + named, patch, err := c.DeclareTypeNamed(name) + if err != nil { return nil, err } - c.mu.Lock() - if c.named == nil { - c.named = make(map[string]*TypeNamed) - } - if typ, ok := c.named[name]; ok { - c.mu.Unlock() - typ.Wait() - if typ.Type != inner { + if patch < 0 { + if named.Type != inner { return nil, fmt.Errorf("type %q redefined", name) } - return typ, nil - - } - c.mu.Unlock() - named, patch, _ := c.DeclareTypeNamed(name) - if patch < 0 { return named, nil } if err := c.BindTypeNamed(patch, named, inner); err != nil {