From 9fce32fe4bf4ba9bdfee7be16c99f672a42dc759 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 d810212d23b..d41d521ef74 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 nil, ipld.ErrNotFound{Cid: 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 57a3b7d6cae..7aae4c350fa 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 a3ca117b22a..57948994770 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 87d739ea95fafde0775a629af356425114bcee0b 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 7aae4c350fa..495c6c4dbcd 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) {