From 385f787ffc66efd6fc5b67e716daa0353144cfe1 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 7 Nov 2022 10:39:31 -0800 Subject: [PATCH 1/2] fix: autobatch: remove potential deadlock when a block is missing Check the _underlying_ blockstore instead of recursing. Also, drop the lock before we do that. --- blockstore/autobatch.go | 8 ++++++-- blockstore/autobatch_test.go | 6 ++++++ blockstore/union_test.go | 1 + 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/blockstore/autobatch.go b/blockstore/autobatch.go index 90f4ad071..d41d521ef 100644 --- a/blockstore/autobatch.go +++ b/blockstore/autobatch.go @@ -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 { diff --git a/blockstore/autobatch_test.go b/blockstore/autobatch_test.go index 57a3b7d6c..7aae4c350 100644 --- a/blockstore/autobatch_test.go +++ b/blockstore/autobatch_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/stretchr/testify/require" + + ipld "github.com/ipfs/go-ipld-format" ) func TestAutobatchBlockstore(t *testing.T) { @@ -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)) } diff --git a/blockstore/union_test.go b/blockstore/union_test.go index a3ca117b2..579489947 100644 --- a/blockstore/union_test.go +++ b/blockstore/union_test.go @@ -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) { From 2e781580992db5bf6eb391f121c7014e13a12ba7 Mon Sep 17 00:00:00 2001 From: Aayush Date: Mon, 7 Nov 2022 15:07:29 -0500 Subject: [PATCH 2/2] fix imports --- blockstore/autobatch_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/blockstore/autobatch_test.go b/blockstore/autobatch_test.go index 7aae4c350..495c6c4db 100644 --- a/blockstore/autobatch_test.go +++ b/blockstore/autobatch_test.go @@ -4,9 +4,8 @@ import ( "context" "testing" - "github.com/stretchr/testify/require" - ipld "github.com/ipfs/go-ipld-format" + "github.com/stretchr/testify/require" ) func TestAutobatchBlockstore(t *testing.T) {