diff --git a/.golangci.yml b/.golangci.yml index 7d49626ca991ee5f238163ade52d90c6d2cfd5ef..f5e601e1469b2d23e6952b1c736bcd8483b36b12 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,8 +14,15 @@ linters: - structcheck - unused - varcheck + - gocritic linters-settings: + gocritic: + enabled-checks: + - ruleguard + settings: + ruleguard: + rules: "rules.go" govet: disable: - deepequalerrors diff --git a/cmd/integration/commands/refetence_db.go b/cmd/integration/commands/refetence_db.go index f589ee0d4bd1d781c53048a81eff951560a8d792..41b27fc574587066ba99c34378eb2e1a4e22cfb1 100644 --- a/cmd/integration/commands/refetence_db.go +++ b/cmd/integration/commands/refetence_db.go @@ -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 diff --git a/cmd/snapshots/debug/debug_test.go b/cmd/snapshots/debug/debug_test.go index 06050d23203827e3afec83124f1ec69d2fbb76da..b1792269945b2cc52f5b646b70111dbbe8451a5c 100644 --- a/cmd/snapshots/debug/debug_test.go +++ b/cmd/snapshots/debug/debug_test.go @@ -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()) diff --git a/cmd/state/commands/check_change_sets.go b/cmd/state/commands/check_change_sets.go index d64cab776f933860a53dd270f947fecf78452f47..8ac516beef93817ce1a78d65f164a390de8b53e0 100644 --- a/cmd/state/commands/check_change_sets.go +++ b/cmd/state/commands/check_change_sets.go @@ -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 { diff --git a/core/state/state_test.go b/core/state/state_test.go index 3c11e5b86112cdac1279ae29182831f6a0066642..aed14f4e32bb9c7e1869408cd3df8aa55a9c51df 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -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) } diff --git a/ethdb/kv/kv_snapshot.go b/ethdb/kv/kv_snapshot.go index 4a3f213ef75f501c135d0d2bdbac04eaa65f6910..94b7c9ba1b8698520c42ce14f7926a192b64f4ea 100644 --- a/ethdb/kv/kv_snapshot.go +++ b/ethdb/kv/kv_snapshot.go @@ -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 } diff --git a/ethdb/kv/kv_snapshot_property_test.go b/ethdb/kv/kv_snapshot_property_test.go index 002b1438f1f7fa2d63299f8cf9297b5f85f8986c..d5138dbf5d2cf0899778d839cfe26fd691c84090 100644 --- a/ethdb/kv/kv_snapshot_property_test.go +++ b/ethdb/kv/kv_snapshot_property_test.go @@ -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 diff --git a/ethdb/kv/memory_database.go b/ethdb/kv/memory_database.go index baed6707fd530768433112964517210eb222b1ec..35e8a1a0d2c82449ab08a9c0a0ee0bab7d6d6fc1 100644 --- a/ethdb/kv/memory_database.go +++ b/ethdb/kv/memory_database.go @@ -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) } diff --git a/go.mod b/go.mod index a3b62ecb80304347d7fa9e6825ee780cb510893a..13e789854e0ff8389c386255562f20f9d23556ad 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 3025ff78f84af419b3ae65a990327c69a3d99215..26d5f5b11026115ee6e6e359f638c9a6fe029124 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/rules.go b/rules.go new file mode 100644 index 0000000000000000000000000000000000000000..ce5d33b535a20c9be19eb7c005824628851c257a --- /dev/null +++ b/rules.go @@ -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.`) +} diff --git a/cmd/hack/binary-deps/main.go b/tools.go similarity index 86% rename from cmd/hack/binary-deps/main.go rename to tools.go index e1560d571787bbaaff57870760c496b37859c132..0eeaaecf4223a271b92a4829726f5f1d1984e7eb 100644 --- a/cmd/hack/binary-deps/main.go +++ b/tools.go @@ -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" diff --git a/turbo/snapshotsync/headers_snapshot.go b/turbo/snapshotsync/headers_snapshot.go index 320a7e139466c9e921ceaa3ec8afc8bf41c1eac5..6bd0dd8815a359d54015646a2f67eabf074ddd84 100644 --- a/turbo/snapshotsync/headers_snapshot.go +++ b/turbo/snapshotsync/headers_snapshot.go @@ -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 {