diff --git a/hamt/hamt.go b/hamt/hamt.go index 593b64627..a1c3d13da 100644 --- a/hamt/hamt.go +++ b/hamt/hamt.go @@ -171,7 +171,11 @@ func (ds *Shard) Node() (ipld.Node, error) { continue } - ch := ds.childer.child(sliceIndex) + chl := ds.childer.childOrLink(sliceIndex) + + ch := chl.child + lnk := chl.link + if ch != nil { clnk, err := ch.Link() if err != nil { @@ -184,7 +188,6 @@ func (ds *Shard) Node() (ipld.Node, error) { } } else { // child unloaded, just copy in link with updated name - lnk := ds.childer.link(sliceIndex) label := lnk.Name[ds.maxpadlen:] err := out.AddRawLink(ds.linkNamePrefix(childIndex)+label, lnk) @@ -415,46 +418,55 @@ type listCidsAndShards struct { func (ds *Shard) walkChildren(processLinkValues func(formattedLink *ipld.Link) error) (*listCidsAndShards, error) { res := &listCidsAndShards{} - for idx, lnk := range ds.childer.links { - if nextShard := ds.childer.children[idx]; nextShard == nil { - lnkLinkType, err := ds.childLinkType(lnk) - if err != nil { + for i := 0; i < ds.childer.length(); i++ { + chl := ds.childer.childOrLink(i) + + lnk := chl.link + nextShard := chl.child + + if nextShard != nil && nextShard.val != nil { + formattedLink := &ipld.Link{ + Name: nextShard.key, + Size: nextShard.val.Size, + Cid: nextShard.val.Cid, + } + if err := processLinkValues(formattedLink); err != nil { return nil, err } - switch lnkLinkType { - case shardValueLink: - sv, err := ds.makeShardValue(lnk) - if err != nil { - return nil, err - } - formattedLink := sv.val - formattedLink.Name = sv.key + continue + } - if err := processLinkValues(formattedLink); err != nil { - return nil, err - } - case shardLink: - res.cids = append(res.cids, lnk.Cid) - default: - return nil, fmt.Errorf("unsupported shard link type") + if nextShard != nil { + res.shards = append(res.shards, nextShard) + + continue + } + + lnkLinkType, err := ds.childLinkType(lnk) + if err != nil { + return nil, err + } + + switch lnkLinkType { + case shardValueLink: + sv, err := ds.makeShardValue(lnk) + if err != nil { + return nil, err } + formattedLink := sv.val + formattedLink.Name = sv.key - } else { - if nextShard.val != nil { - formattedLink := &ipld.Link{ - Name: nextShard.key, - Size: nextShard.val.Size, - Cid: nextShard.val.Cid, - } - if err := processLinkValues(formattedLink); err != nil { - return nil, err - } - } else { - res.shards = append(res.shards, nextShard) + if err := processLinkValues(formattedLink); err != nil { + return nil, err } + case shardLink: + res.cids = append(res.cids, lnk.Cid) + default: + return nil, fmt.Errorf("unsupported shard link type") } } + return res, nil } @@ -707,7 +719,11 @@ func (ds *Shard) swapValue(ctx context.Context, hv *hashBits, key string, value // shard in case an implementation is broken. // Have we loaded the child? Prefer that. - schild := child.childer.child(0) + chl := child.childer.childOrLink(0) + + schild := chl.child + slnk := chl.link + if schild != nil { if schild.isValueNode() { ds.childer.set(schild, i) @@ -716,7 +732,6 @@ func (ds *Shard) swapValue(ctx context.Context, hv *hashBits, key string, value } // Otherwise, work with the link. - slnk := child.childer.link(0) var lnkType linkType lnkType, err = child.childer.sd.childLinkType(slnk) if err != nil { @@ -750,9 +765,14 @@ type childer struct { dserv ipld.DAGService bitfield bitfield.Bitfield - // Only one of links/children will be non-nil for every child/link. - links []*ipld.Link - children []*Shard + mu sync.RWMutex + children []childOrLink +} + +// childOrLink is a data structure that will contains a link OR a child. +type childOrLink struct { + link *ipld.Link + child *Shard } func newChilder(ds ipld.DAGService, size int) *childer { @@ -763,11 +783,13 @@ func newChilder(ds ipld.DAGService, size int) *childer { } func (s *childer) makeChilder(data []byte, links []*ipld.Link) *childer { - s.children = make([]*Shard, len(links)) + s.mu.Lock() + defer s.mu.Unlock() + s.bitfield.SetBytes(data) - if len(links) > 0 { - s.links = make([]*ipld.Link, len(links)) - copy(s.links, links) + + for _, l := range links { + s.children = append(s.children, childOrLink{link: l}) } return s @@ -778,12 +800,11 @@ func (s *childer) sliceIndex(childIndex int) (sliceIndex int) { return s.bitfield.OnesBefore(childIndex) } -func (s *childer) child(sliceIndex int) *Shard { - return s.children[sliceIndex] -} +func (s *childer) childOrLink(sliceIndex int) childOrLink { + s.mu.RLock() + defer s.mu.RUnlock() -func (s *childer) link(sliceIndex int) *ipld.Link { - return s.links[sliceIndex] + return s.children[sliceIndex] } func (s *childer) insert(key string, lnk *ipld.Link, idx int) error { @@ -795,8 +816,12 @@ func (s *childer) insert(key string, lnk *ipld.Link, idx int) error { i := s.sliceIndex(idx) sd := &Shard{key: key, val: lnk} - s.children = append(s.children[:i], append([]*Shard{sd}, s.children[i:]...)...) - s.links = append(s.links[:i], append([]*ipld.Link{nil}, s.links[i:]...)...) + s.mu.Lock() + + s.children = append(s.children[:i], append([]childOrLink{{child: sd}}, s.children[i:]...)...) + + s.mu.Unlock() + // Add a `nil` placeholder in `links` so the rest of the entries keep the same // index as `children`. s.bitfield.SetBit(idx) @@ -805,13 +830,17 @@ func (s *childer) insert(key string, lnk *ipld.Link, idx int) error { } func (s *childer) set(sd *Shard, i int) { - s.children[i] = sd - s.links[i] = nil + s.mu.Lock() + defer s.mu.Unlock() + + s.children[i] = childOrLink{child: sd} } func (s *childer) setLink(lnk *ipld.Link, i int) { - s.children[i] = nil - s.links[i] = lnk + s.mu.Lock() + defer s.mu.Unlock() + + s.children[i] = childOrLink{link: lnk} } func (s *childer) rm(childIndex int) error { @@ -821,11 +850,12 @@ func (s *childer) rm(childIndex int) error { return err } + s.mu.Lock() + copy(s.children[i:], s.children[i+1:]) s.children = s.children[:len(s.children)-1] - copy(s.links[i:], s.links[i+1:]) - s.links = s.links[:len(s.links)-1] + s.mu.Unlock() s.bitfield.UnsetBit(childIndex) @@ -840,18 +870,17 @@ func (s *childer) get(ctx context.Context, sliceIndex int) (*Shard, error) { return nil, err } - c := s.child(sliceIndex) - if c != nil { - return c, nil + c := s.childOrLink(sliceIndex) + if c.child != nil { + return c.child, nil } - return s.loadChild(ctx, sliceIndex) + return s.loadChild(ctx, c.link, sliceIndex) } // loadChild reads the i'th child node of this shard from disk and returns it // as a 'child' interface -func (s *childer) loadChild(ctx context.Context, sliceIndex int) (*Shard, error) { - lnk := s.link(sliceIndex) +func (s *childer) loadChild(ctx context.Context, lnk *ipld.Link, sliceIndex int) (*Shard, error) { lnkLinkType, err := s.sd.childLinkType(lnk) if err != nil { return nil, err @@ -910,9 +939,5 @@ func (s *childer) check(sliceIndex int) error { return fmt.Errorf("invalid index passed to operate children (likely corrupt bitfield)") } - if len(s.children) != len(s.links) { - return fmt.Errorf("inconsistent lengths between children array and Links array") - } - return nil } diff --git a/hamt/hamt_test.go b/hamt/hamt_test.go index 8d0b93889..15b28f57a 100644 --- a/hamt/hamt_test.go +++ b/hamt/hamt_test.go @@ -6,6 +6,7 @@ import ( "math/rand" "os" "sort" + "sync" "testing" "time" @@ -710,3 +711,32 @@ func TestHamtBadSize(t *testing.T) { t.Fatal("should have failed to construct hamt with bad size") } } + +func TestChilderModifyChildrenAndLinks(t *testing.T) { + ds := mdtest.Mock() + sh, _ := NewShard(ds, 256) + + ch := newChilder(ds, 1024) + ch.makeChilder(nil, []*ipld.Link{{}}) + + var wg sync.WaitGroup + for i := 0; i < 100000; i++ { + wg.Add(2) + go func() { + ch.set(sh, 0) + wg.Done() + }() + go func() { + ch.setLink(&ipld.Link{}, 0) + wg.Done() + }() + + chl := ch.childOrLink(0) + if chl.child == nil && chl.link == nil { + t.Fatal("both slices are nil", i) + } + + } + + wg.Wait() +}