Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linter: to check tx.Rollback() by ruleguard #2383

Merged
merged 5 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ci to check rollback
  • Loading branch information
AskAlexSharov committed Jul 16, 2021
commit 3effefbc2f796e84c8a38950af44d16b02bd6c50
21 changes: 2 additions & 19 deletions cmd/integration/commands/refetence_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,14 +339,6 @@ MainLoop:
if err != nil {
return err
}
dstTx, err = dst.BeginRw(ctx)
if err != nil {
return err
}
err = dstTx.Commit()
if err != nil {
return err
}

return nil
}
Expand All @@ -368,9 +360,7 @@ func kv2kv(ctx context.Context, src, dst ethdb.RwKV) error {
if err1 != nil {
return err1
}
defer func() {
dstTx.Rollback()
}()
defer dstTx.Rollback()

commitEvery := time.NewTicker(30 * time.Second)
defer commitEvery.Stop()
Expand Down Expand Up @@ -417,6 +407,7 @@ func kv2kv(ctx context.Context, src, dst ethdb.RwKV) error {
if err != nil {
return err
}
defer dstTx.Rollback()
c, err = dstTx.RwCursor(name)
if err != nil {
return err
Expand All @@ -440,14 +431,6 @@ func kv2kv(ctx context.Context, src, dst ethdb.RwKV) error {
if err != nil {
return err
}
dstTx, err = dst.BeginRw(ctx)
if err != nil {
return err
}
err = dstTx.Commit()
if err != nil {
return err
}
srcTx.Rollback()
log.Info("done")
return nil
Expand Down
1 change: 1 addition & 0 deletions cmd/snapshots/debug/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ func TestMatreshkaStream(t *testing.T) {
if err != nil {
t.Fatal(err, currentBlock)
}
defer tx.Rollback()

dr := time.Since(ttt)
fmt.Println(currentBlock, "finished", "acc-", accDiffLen, "st-", stDiffLen, "codes - ", codesDiffLen, "all -", time.Since(tt), "chunk - ", dr, "blocks/s", 10000/dr.Seconds())
Expand Down
1 change: 1 addition & 0 deletions cmd/state/commands/check_change_sets.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ func CheckChangeSets(genesis *core.Genesis, blockNum uint64, chaindata string, h
return err
}
rwtx, err = chainDb.BeginRw(context.Background())
defer rwtx.Rollback()
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion core/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (s *StateSuite) TestDump(c *checker.C) {

func (s *StateSuite) SetUpTest(c *checker.C) {
s.kv = kv.NewMemKV()
tx, err := s.kv.BeginRw(context.Background())
tx, err := s.kv.BeginRw(context.Background()) //nolint
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion ethdb/kv/kv_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (s *SnapshotKV) BeginRo(ctx context.Context) (ethdb.Tx, error) {
}

func (s *SnapshotKV) BeginRw(ctx context.Context) (ethdb.RwTx, error) {
dbTx, err := s.db.BeginRw(ctx)
dbTx, err := s.db.BeginRw(ctx) //nolint
if err != nil {
return nil, err
}
Expand Down
10 changes: 6 additions & 4 deletions ethdb/kv/kv_snapshot_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func (m *getPutkvMachine) Init(t *rapid.T) {

txSn, err := m.snKV.BeginRw(context.Background())
require.NoError(t, err)
defer txSn.Rollback()

txModel, err := m.modelKV.BeginRw(context.Background())
require.NoError(t, err)
Expand Down Expand Up @@ -133,9 +134,9 @@ func (m *getPutkvMachine) Begin(t *rapid.T) {
if m.modelTX != nil && m.snTX != nil {
return
}
mtx, err := m.modelKV.BeginRw(context.Background())
mtx, err := m.modelKV.BeginRw(context.Background()) //nolint
require.NoError(t, err)
sntx, err := m.snKV.BeginRw(context.Background())
sntx, err := m.snKV.BeginRw(context.Background()) //nolint
require.NoError(t, err)
m.modelTX = mtx
m.snTX = sntx
Expand Down Expand Up @@ -189,6 +190,7 @@ func (m *getKVMachine) Init(t *rapid.T) {

txSn, err := m.snKV.BeginRw(context.Background())
require.NoError(t, err)
defer txSn.Rollback()

txModel, err := m.modelKV.BeginRw(context.Background())
require.NoError(t, err)
Expand Down Expand Up @@ -343,9 +345,9 @@ func (m *cursorKVMachine) Begin(t *rapid.T) {
return
}

mtx, err := m.modelKV.BeginRw(context.Background())
mtx, err := m.modelKV.BeginRw(context.Background()) //nolint
require.NoError(t, err)
sntx, err := m.snKV.BeginRw(context.Background())
sntx, err := m.snKV.BeginRw(context.Background()) //nolint
require.NoError(t, err)
m.modelTX = mtx
m.snTX = sntx
Expand Down
2 changes: 1 addition & 1 deletion ethdb/kv/memory_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func NewTestTx(t testing.TB) (ethdb.RwKV, ethdb.RwTx) {
tt.Cleanup(kv.Close)
}
}
tx, err := kv.BeginRw(context.Background())
tx, err := kv.BeginRw(context.Background()) //nolint
if err != nil {
t.Fatal(err)
}
Expand Down
5 changes: 4 additions & 1 deletion rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ func txDeferRollback(m dsl.Matcher) {
).
Where(!m["rollback"].Text.Matches(`defer .*\.Rollback()`)).
//At(m["unlock"]).
Report(`add "defer $tx.Rollback()" right after transaction creation error check. Or consider use "$db.View" or "$db.Update" .`)
Report(`Add "defer $tx.Rollback()" right after transaction creation error check.
If you are in the loop - consider use "$db.View" or "$db.Update" or extract whole transaction to function.
Without rollback in defer - app can deadlock on error or panic.
`)

}

Expand Down
27 changes: 19 additions & 8 deletions turbo/snapshotsync/snapshot_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,16 +485,11 @@ func TestSnapshotMigratorStageSyncMode(t *testing.T) {

tm := time.After(time.Second * 10)
for atomic.LoadUint64(&sb.started) > 0 && atomic.LoadUint64(&sb.HeadersCurrentSnapshot) != 10 {
roTx, err := db.BeginRo(context.Background())
err = db.View(context.Background(), func(tx ethdb.Tx) error { return sb.Final(tx) })
if err != nil {
t.Fatal(err)
}

err = sb.Final(roTx)
if err != nil {
t.Fatal(err)
}
roTx.Rollback()
select {
case <-tm:
t.Fatal("timeout")
Expand Down Expand Up @@ -1087,8 +1082,8 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}

defer tx.Rollback()

dataTo := uint64(15)
snapshotTo := uint64(10)
err = GenerateBodyData(tx, 0, dataTo)
Expand All @@ -1105,6 +1100,7 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer readTX.Rollback()

bodySnapshotPath := filepath.Join(snapshotsDir, SnapshotName(snapshotsDir, "bodies", snapshotTo))
err = CreateBodySnapshot(readTX, snapshotTo, bodySnapshotPath)
Expand All @@ -1121,6 +1117,7 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer bodySnapshotTX.Rollback()

verifyBodiesSnapshot(t, bodySnapshotTX, snapshotTo)
ethTXCursor, err := bodySnapshotTX.Cursor(dbutils.EthTx)
Expand All @@ -1131,8 +1128,8 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}

bodySnapshotTX.Rollback()

ch := make(chan struct{})
db.UpdateSnapshots("bodies", kvSnapshot, ch)
select {
Expand All @@ -1144,13 +1141,16 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer withBodySnapshotTX.Rollback()

verifyFullBodiesData(t, withBodySnapshotTX, dataTo)
withBodySnapshotTX.Rollback()

rwTX, err := db.BeginRw(context.Background())
if err != nil {
t.Fatal(err)
}
defer rwTX.Rollback()

err = RemoveBlocksData(db, rwTX, snapshotTo)
if err != nil {
Expand All @@ -1166,6 +1166,8 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer withBodySnapshotTX.Rollback()

verifyFullBodiesData(t, withBodySnapshotTX, dataTo)
withBodySnapshotTX.Rollback()

Expand All @@ -1175,13 +1177,16 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer writeDBKVRoTX.Rollback()

verifyPrunedBlocksData(t, writeDBKVRoTX, snapshotTo, dataTo, binary.BigEndian.Uint64(lastTxID))
writeDBKVRoTX.Rollback()

tx, err = db.BeginRw(context.Background())
if err != nil {
t.Fatal(err)
}
defer tx.Rollback()

//snapshot to 20
dataFrom := uint64(16)
Expand All @@ -1202,6 +1207,7 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer readTX.Rollback()

err = CreateBodySnapshot(readTX, snapshotTo, bodySnapshotPath)
if err != nil {
Expand All @@ -1218,6 +1224,7 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer bodySnapshotTX.Rollback()

verifyBodiesSnapshot(t, bodySnapshotTX, snapshotTo)
ethTXCursor, err = bodySnapshotTX.Cursor(dbutils.EthTx)
Expand All @@ -1241,13 +1248,16 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer withBodySnapshotTX.Rollback()

verifyFullBodiesData(t, withBodySnapshotTX, dataTo)
withBodySnapshotTX.Rollback()

rwTX, err = db.BeginRw(context.Background())
if err != nil {
t.Fatal(err)
}
defer rwTX.Rollback()

err = RemoveBlocksData(db, rwTX, snapshotTo)
if err != nil {
Expand All @@ -1263,6 +1273,7 @@ func TestPruneBlocks(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer withBodySnapshotTX.Rollback()

verifyFullBodiesData(t, withBodySnapshotTX, dataTo)
withBodySnapshotTX.Rollback()
Expand Down