diff --git a/.golangci.yaml b/.golangci.yaml index 0bdc00e4ed..288653e7ab 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -83,7 +83,7 @@ linters: - ruleguard settings: ruleguard: - rules: './analysis/ssrf.go,./analysis/git.go' + rules: './analysis/ssrf.go,./analysis/git.go,./analysis/transaction.go' forbidigo: forbid: - pattern: ^tls\.Config$ diff --git a/analysis/transaction.go b/analysis/transaction.go new file mode 100644 index 0000000000..f229f62de1 --- /dev/null +++ b/analysis/transaction.go @@ -0,0 +1,37 @@ +//go:build ignore + +package gorules + +import "github.com/quasilyte/go-ruleguard/dsl" + +// nestedDBTransaction flags a DataStore transaction started from inside another +// transaction closure. bbolt only allows one open write transaction at a time, +// so starting a second one while the first is still open deadlocks. This fires +// regardless of whether the inner transaction is opened through the same store +// variable or a different one, since there is only one underlying database. +func nestedDBTransaction(m dsl.Matcher) { + m.Match( + `$store.UpdateTx($fn)`, + `$store.UpdateTxLowPriority($fn)`, + `$store.ViewTx($fn)`, + ). + Where( + m["fn"].Contains(`$_.UpdateTx($_)`) || + m["fn"].Contains(`$_.UpdateTxLowPriority($_)`) || + m["fn"].Contains(`$_.ViewTx($_)`)). + Report(`$fn starts another transaction while already running inside $store's transaction; only one write transaction can be open at a time, so this can deadlock`) +} + +// dataStoreCallInsideTx flags a service accessed through the DataStore instead +// of the tx handle inside a transaction closure. The non-tx CRUD methods open +// their own transaction internally (see BaseDataService), so calling them from +// inside an already-open transaction is a nested transaction in disguise. +func dataStoreCallInsideTx(m dsl.Matcher) { + m.Match( + `$store.UpdateTx($fn)`, + `$store.UpdateTxLowPriority($fn)`, + `$store.ViewTx($fn)`, + ). + Where(m["fn"].Contains(`$store.$service().$method($*_)`)). + Report(`$service() is accessed through $store instead of the tx parameter inside this closure; the non-tx call starts its own transaction internally and can deadlock`) +} diff --git a/api/http/handler/endpointgroups/endpointgroup_list.go b/api/http/handler/endpointgroups/endpointgroup_list.go index a574905f81..9a356d5a88 100644 --- a/api/http/handler/endpointgroups/endpointgroup_list.go +++ b/api/http/handler/endpointgroups/endpointgroup_list.go @@ -95,7 +95,7 @@ func (handler *Handler) endpointGroupList(w http.ResponseWriter, r *http.Request var handlerErr *httperror.HandlerError if err := handler.DataStore.ViewTx(func(tx dataservices.DataStoreTx) error { var txErr error - endpointGroups, txErr = handler.DataStore.EndpointGroup().ReadAll() + endpointGroups, txErr = tx.EndpointGroup().ReadAll() if txErr != nil { handlerErr = httperror.InternalServerError("Unable to retrieve environment groups from the database", txErr) return handlerErr