diff --git a/store/snapshots/store.go b/store/snapshots/store.go index 2f08a6e6c4..aa593f5bac 100644 --- a/store/snapshots/store.go +++ b/store/snapshots/store.go @@ -3,6 +3,7 @@ package snapshots import ( "crypto/sha256" "encoding/binary" + "fmt" "hash" "io" "math" @@ -140,6 +141,7 @@ func (s *Store) Load(height uint64, format uint32) (*types.Snapshot, <-chan io.R } ch := make(chan io.ReadCloser) + go func() { defer close(ch) for i := uint32(0); i < snapshot.Chunks; i++ { @@ -150,14 +152,19 @@ func (s *Store) Load(height uint64, format uint32) (*types.Snapshot, <-chan io.R _ = pw.CloseWithError(err) return } - defer chunk.Close() - _, err = io.Copy(pw, chunk) + err = func() error { + defer chunk.Close() + + if _, err := io.Copy(pw, chunk); err != nil { + _ = pw.CloseWithError(err) + return fmt.Errorf("failed to copy chunk %d: %w", i, err) + } + + return pw.Close() + }() if err != nil { - _ = pw.CloseWithError(err) return } - chunk.Close() - pw.Close() } }() diff --git a/store/v2/commitment/store.go b/store/v2/commitment/store.go index a18cbc49e3..46ea503b2b 100644 --- a/store/v2/commitment/store.go +++ b/store/v2/commitment/store.go @@ -411,7 +411,9 @@ loop: if err := importer.Commit(); err != nil { return snapshotstypes.SnapshotItem{}, fmt.Errorf("failed to commit importer: %w", err) } - importer.Close() + if err := importer.Close(); err != nil { + return snapshotstypes.SnapshotItem{}, fmt.Errorf("failed to close importer: %w", err) + } } storeKey = []byte(item.Store.Name) diff --git a/store/v2/migration/manager.go b/store/v2/migration/manager.go index 4a719c06f4..c080cb55cb 100644 --- a/store/v2/migration/manager.go +++ b/store/v2/migration/manager.go @@ -181,11 +181,21 @@ func (m *Manager) writeChangeset() error { } batch := m.db.NewBatch() - if err := batch.Set(csKey, csBytes); err != nil { - return fmt.Errorf("failed to write changeset to db.Batch: %w", err) - } - if err := batch.Write(); err != nil { - return fmt.Errorf("failed to write changeset to db: %w", err) + // Invoking this code in a closure so that defer is called immediately on return + // yet not in the for-loop which can leave resource lingering. + err = func() error { + defer batch.Close() + + if err := batch.Set(csKey, csBytes); err != nil { + return fmt.Errorf("failed to write changeset to db.Batch: %w", err) + } + if err := batch.Write(); err != nil { + return fmt.Errorf("failed to write changeset to db: %w", err) + } + return nil + }() + if err != nil { + return err } batch.Close() } diff --git a/store/v2/storage/storage_test_suite.go b/store/v2/storage/storage_test_suite.go index b7d748386e..be5ea003f8 100644 --- a/store/v2/storage/storage_test_suite.go +++ b/store/v2/storage/storage_test_suite.go @@ -256,8 +256,6 @@ func (s *StorageTestSuite) TestDatabase_Iterator() { itr, err := db.Iterator(storeKey1Bytes, v, []byte("key000"), nil) s.Require().NoError(err) - defer itr.Close() - var i, count int for ; itr.Valid(); itr.Next() { s.Require().Equal([]byte(fmt.Sprintf("key%03d", i)), itr.Key(), string(itr.Key())) @@ -271,6 +269,9 @@ func (s *StorageTestSuite) TestDatabase_Iterator() { // seek past domain, which should make the iterator invalid and produce an error s.Require().False(itr.Valid()) + + err = itr.Close() + s.Require().NoError(err, "Failed to close iterator") } // iterator with a start and end domain over multiple versions @@ -278,8 +279,6 @@ func (s *StorageTestSuite) TestDatabase_Iterator() { itr2, err := db.Iterator(storeKey1Bytes, v, []byte("key010"), []byte("key019")) s.Require().NoError(err) - defer itr2.Close() - i, count := 10, 0 for ; itr2.Valid(); itr2.Next() { s.Require().Equal([]byte(fmt.Sprintf("key%03d", i)), itr2.Key()) @@ -293,6 +292,11 @@ func (s *StorageTestSuite) TestDatabase_Iterator() { // seek past domain, which should make the iterator invalid and produce an error s.Require().False(itr2.Valid()) + + err = itr2.Close() + if err != nil { + return + } } // start must be <= end diff --git a/tests/integration/auth/client/cli/suite_test.go b/tests/integration/auth/client/cli/suite_test.go index 15eeb31e42..8c87e6b27c 100644 --- a/tests/integration/auth/client/cli/suite_test.go +++ b/tests/integration/auth/client/cli/suite_test.go @@ -207,7 +207,6 @@ func (s *CLITestSuite) TestCLISignBatchTotalFees() { sdk.NewCoins(sendTokens), clitestutil.TestTxConfig{GenOnly: true}) s.Require().NoError(err) txFile := testutil.WriteToNewTempFile(s.T(), tx.String()+"\n") - defer txFile.Close() txFiles[i] = txFile.Name() unsignedTx, err := txCfg.TxJSONDecoder()(tx.Bytes()) @@ -215,6 +214,8 @@ func (s *CLITestSuite) TestCLISignBatchTotalFees() { txBuilder, err := txCfg.WrapTxBuilder(unsignedTx) s.Require().NoError(err) expectedBatchedTotalFee += txBuilder.GetTx().GetFee().AmountOf(tc.denom).Int64() + err = txFile.Close() + s.NoError(err) } // Test batch sign diff --git a/x/group/keeper/invariants.go b/x/group/keeper/invariants.go index a88f6d1069..d3f47201a2 100644 --- a/x/group/keeper/invariants.go +++ b/x/group/keeper/invariants.go @@ -55,7 +55,6 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, storeService storetypes.KV msg += fmt.Sprintf("LoadNext failure on group table iterator\n%v\n", err) return msg, broken } - groups[groupInfo.Id] = groupInfo } @@ -63,6 +62,7 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, storeService storetypes.KV sort.Slice(groupByIDs, func(i, j int) bool { return groupByIDs[i] < groupByIDs[j] }) + for _, groupID := range groupByIDs { groupInfo := groups[groupID] membersWeight, err := groupmath.NewNonNegativeDecFromString("0") @@ -71,36 +71,39 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, storeService storetypes.KV return msg, broken } - memIt, err := groupMemberByGroupIndex.Get(kvStore, groupInfo.Id) + err = func() error { + memIt, err := groupMemberByGroupIndex.Get(kvStore, groupInfo.Id) + if err != nil { + return fmt.Errorf("error while returning group member iterator for group with ID %d\n%w", groupInfo.Id, err) + } + defer memIt.Close() + + for { + var groupMember group.GroupMember + _, err = memIt.LoadNext(&groupMember) + if errors.ErrORMIteratorDone.Is(err) { + break + } + if err != nil { + return fmt.Errorf("LoadNext failure on member table iterator\n%w", err) + } + + curMemWeight, err := groupmath.NewPositiveDecFromString(groupMember.GetMember().GetWeight()) + if err != nil { + return fmt.Errorf("error while parsing non-nengative decimal for group member %s\n%w", groupMember.Member.Address, err) + } + + membersWeight, err = groupmath.Add(membersWeight, curMemWeight) + if err != nil { + return fmt.Errorf("decimal addition error while adding group member voting weight to total voting weight\n%w", err) + } + } + return nil + }() if err != nil { - msg += fmt.Sprintf("error while returning group member iterator for group with ID %d\n%v\n", groupInfo.Id, err) + msg += err.Error() + "\n" return msg, broken } - defer memIt.Close() - - for { - var groupMember group.GroupMember - _, err = memIt.LoadNext(&groupMember) - if errors.ErrORMIteratorDone.Is(err) { - break - } - if err != nil { - msg += fmt.Sprintf("LoadNext failure on member table iterator\n%v\n", err) - return msg, broken - } - - curMemWeight, err := groupmath.NewPositiveDecFromString(groupMember.GetMember().GetWeight()) - if err != nil { - msg += fmt.Sprintf("error while parsing non-nengative decimal for group member %s\n%v\n", groupMember.Member.Address, err) - return msg, broken - } - - membersWeight, err = groupmath.Add(membersWeight, curMemWeight) - if err != nil { - msg += fmt.Sprintf("decimal addition error while adding group member voting weight to total voting weight\n%v\n", err) - return msg, broken - } - } groupWeight, err := groupmath.NewNonNegativeDecFromString(groupInfo.GetTotalWeight()) if err != nil {