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

This commit is contained in:
Alex Sharov 2021-07-16 20:23:54 +07:00 committed by GitHub
parent 1cb962c6e4
commit 9f6ef74adc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 100 additions and 36 deletions

View File

@ -14,8 +14,15 @@ linters:
- structcheck
- unused
- varcheck
- gocritic
linters-settings:
gocritic:
enabled-checks:
- ruleguard
settings:
ruleguard:
rules: "rules.go"
govet:
disable:
- deepequalerrors

View File

@ -255,9 +255,7 @@ func fToMdbx(ctx context.Context, to string) error {
if err1 != nil {
return err1
}
defer func() {
dstTx.Rollback()
}()
defer dstTx.Rollback()
commitEvery := time.NewTicker(5 * time.Second)
defer commitEvery.Stop()
@ -341,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
}
@ -370,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()
@ -419,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
@ -442,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

View File

@ -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())

View File

@ -230,6 +230,7 @@ func CheckChangeSets(genesis *core.Genesis, blockNum uint64, chaindata string, h
if err != nil {
return err
}
defer rwtx.Rollback()
historyTx.Rollback()
historyTx, err = historyDb.BeginRo(context.Background())
if err != nil {

View File

@ -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)
}

View File

@ -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
}

View File

@ -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)
@ -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
@ -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)
@ -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

View File

@ -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)
}

1
go.mod
View File

@ -49,6 +49,7 @@ require (
github.com/petar/GoLLRB v0.0.0-20190514000832-33fb24c13b99
github.com/prometheus/client_golang v1.9.0
github.com/prometheus/tsdb v0.10.0
github.com/quasilyte/go-ruleguard/dsl v0.3.6
github.com/rs/cors v1.8.0
github.com/shirou/gopsutil/v3 v3.21.6
github.com/spf13/cobra v1.1.3

4
go.sum
View File

@ -870,6 +870,8 @@ github.com/prometheus/tsdb v0.6.2-0.20190402121629-4f204dcbc150/go.mod h1:qhTCs0
github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
github.com/prometheus/tsdb v0.10.0 h1:If5rVCMTp6W2SiRAQFlbpJNgVlgMEd+U2GZckwK38ic=
github.com/prometheus/tsdb v0.10.0/go.mod h1:oi49uRhEe9dPUTlS3JRZOwJuVi6tmh10QSgwXEyGCt4=
github.com/quasilyte/go-ruleguard/dsl v0.3.6 h1:W2wnISJifyda0x/RXq15Qjrsu9iOhX2gy4+Ku+owylw=
github.com/quasilyte/go-ruleguard/dsl v0.3.6/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/rjeczalik/notify v0.9.1/go.mod h1:rKwnCoCGeuQnwBtTSPL9Dad03Vh2n40ePRrjvIXnJho=
github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
@ -988,8 +990,6 @@ github.com/tklauser/numcpus v0.2.2 h1:oyhllyrScuYI6g+h/zUvNXNp1wy7x8qQy3t/piefld
github.com/tklauser/numcpus v0.2.2/go.mod h1:x3qojaO3uyYt0i56EW/VUYs7uBvdl2fkfZFu0T9wgjM=
github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/torquem-ch/mdbx-go v0.15.0 h1:HUPA06n7tEAyLXv+rFuWvk01FKEm2MavLD9VVEQqMCs=
github.com/torquem-ch/mdbx-go v0.15.0/go.mod h1:T2fsoJDVppxfAPTLd1svUgH1kpPmeXdPESmroSHcL1E=
github.com/torquem-ch/mdbx-go v0.16.0 h1:x4dTXDvy1w9MxjlAX6J3/bDqIqu4XQDqTR/u2jMnTiA=
github.com/torquem-ch/mdbx-go v0.16.0/go.mod h1:T2fsoJDVppxfAPTLd1svUgH1kpPmeXdPESmroSHcL1E=
github.com/ttacon/chalk v0.0.0-20160626202418-22c06c80ed31/go.mod h1:onvgF043R+lC5RZ8IT9rBXDaEDnpnw/Cl+HFiw+v/7Q=

68
rules.go Normal file
View File

@ -0,0 +1,68 @@
// +build gorules
package gorules
// https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
import (
"github.com/quasilyte/go-ruleguard/dsl"
//quasilyterules "github.com/quasilyte/ruleguard-rules-test"
)
func init() {
//dsl.ImportRules("qrules", quasilyterules.Bundle)
}
func txDeferRollback(m dsl.Matcher) {
// Common pattern for long-living transactions:
// tx, err := db.Begin()
// if err != nil {
// return err
// }
// defer tx.Rollback()
//
// ... code which uses database in transaction
//
// err := tx.Commit()
// if err != nil {
// return err
// }
m.Match(
`$tx, $err := $db.BeginRw($ctx); $chk; $rollback`,
`$tx, $err = $db.BeginRw($ctx); $chk; $rollback`,
`$tx, $err := $db.Begin($ctx); $chk; $rollback`,
`$tx, $err = $db.Begin($ctx); $chk; $rollback`,
).
Where(!m["rollback"].Text.Matches(`defer .*\.Rollback()`)).
//At(m["unlock"]).
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.
Rules are in ./rules.go file.
`)
}
func mismatchingUnlock(m dsl.Matcher) {
// By default, an entire match position is used as a location.
// This can be changed by the At() method that binds the location
// to the provided named submatch.
//
// In the rules below text editor would get mismatching method
// name locations:
//
// defer mu.RUnlock()
// ^^^^^^^
m.Match(`$mu.Lock(); defer $mu.$unlock()`).
Where(m["unlock"].Text == "RUnlock").
At(m["unlock"]).
Report(`maybe $mu.Unlock() was intended?
Rules are in ./rules.go file.`)
m.Match(`$mu.RLock(); defer $mu.$unlock()`).
Where(m["unlock"].Text == "Unlock").
At(m["unlock"]).
Report(`maybe $mu.RUnlock() was intended?
Rules are in ./rules.go file.`)
}

View File

@ -1,5 +1,9 @@
// +build trick_go_mod_tidy
// +build tools
package tools
// https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
//
// This module is just a hack for 'go mod tidy' command
//
// Problem is - 'go mod tidy' removes from go.mod file lines if you don't use them in source code
@ -12,8 +16,6 @@
// build tag 'trick_go_mod_tidy' - is used to hide warnings of IDEA (because we can't import `main` packages in go)
package main
import (
_ "github.com/fjl/gencodec"
_ "github.com/kevinburke/go-bindata"

View File

@ -4,14 +4,15 @@ import (
"context"
"errors"
"fmt"
"os"
"time"
"github.com/ledgerwatch/erigon/common"
"github.com/ledgerwatch/erigon/common/dbutils"
"github.com/ledgerwatch/erigon/core/rawdb"
"github.com/ledgerwatch/erigon/ethdb"
"github.com/ledgerwatch/erigon/ethdb/kv"
"github.com/ledgerwatch/erigon/log"
"os"
"time"
)
func CreateHeadersSnapshot(ctx context.Context, readTX ethdb.Tx, toBlock uint64, snapshotPath string) error {