Skip to content

Commit

Permalink
fix: autobatch: remove potential deadlock when a block is missing
Browse files Browse the repository at this point in the history
Check the _underlying_ blockstore instead of recursing. Also, drop the
lock before we do that.
  • Loading branch information
Stebalien committed Nov 7, 2022
1 parent abfabd2 commit 385f787
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
8 changes: 6 additions & 2 deletions blockstore/autobatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,18 +181,22 @@ func (bs *AutobatchBlockstore) Get(ctx context.Context, c cid.Cid) (block.Block,
}

bs.stateLock.Lock()
defer bs.stateLock.Unlock()
v, ok := bs.flushingBatch.blockMap[c]
if ok {
bs.stateLock.Unlock()
return v, nil
}

v, ok = bs.bufferedBatch.blockMap[c]
if ok {
bs.stateLock.Unlock()
return v, nil
}
bs.stateLock.Unlock()

return bs.Get(ctx, c)
// We have to check the backing store one more time because it may have been flushed by the
// time we were able to take the lock above.
return bs.backingBs.Get(ctx, c)
}

func (bs *AutobatchBlockstore) DeleteBlock(context.Context, cid.Cid) error {
Expand Down
6 changes: 6 additions & 0 deletions blockstore/autobatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"

"github.com/stretchr/testify/require"

ipld "github.com/ipfs/go-ipld-format"
)

func TestAutobatchBlockstore(t *testing.T) {
Expand All @@ -29,6 +31,10 @@ func TestAutobatchBlockstore(t *testing.T) {
require.NoError(t, err)
require.Equal(t, b2.RawData(), v2.RawData())

// Regression test for a deadlock.
_, err = ab.Get(ctx, b3.Cid())
require.True(t, ipld.IsNotFound(err))

require.NoError(t, ab.Flush(ctx))
require.NoError(t, ab.Shutdown(ctx))
}
1 change: 1 addition & 0 deletions blockstore/union_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var (
b0 = blocks.NewBlock([]byte("abc"))
b1 = blocks.NewBlock([]byte("foo"))
b2 = blocks.NewBlock([]byte("bar"))
b3 = blocks.NewBlock([]byte("baz"))
)

func TestUnionBlockstore_Get(t *testing.T) {
Expand Down

0 comments on commit 385f787

Please sign in to comment.