Skip to content

Commit

Permalink
Fix goroutine leak v2 (#104)
Browse files Browse the repository at this point in the history
  • Loading branch information
maranqz authored Feb 14, 2024
1 parent f4be550 commit 334154e
Show file tree
Hide file tree
Showing 54 changed files with 1,339 additions and 648 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ env:
OS_TARGET: ubuntu-latest
jobs:
tests-units:
needs: [lint]
permissions:
pull-requests: write
strategy:
Expand Down Expand Up @@ -52,6 +53,7 @@ jobs:
run: make test

tests-integration:
needs: [tests-units]
permissions:
pull-requests: write
services:
Expand Down Expand Up @@ -145,7 +147,7 @@ jobs:
with:
go-version: ${{ env.GO_TARGET_VERSION }}
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
uses: golangci/golangci-lint-action@v4
with:
args: --timeout=3m -v
working-directory: ${{ matrix.modules }}
4 changes: 4 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ linters:
- whitespace
- wsl # https://github.com/bombsimon/wsl/blob/master/doc/rules.md

linters-settings:
wsl:
allow-assign-and-anything: true

issues:
exclude-use-default: false
exclude:
Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [2.0.0-rc-8] - 2024-01-04

### Fixed

- gouroutine leak.

## [2.0.0-rc-7] - 2024-01-04

### Changes

- Transferred drivers in separated modules.

## [1.4.0] - 2023-09-01

### Added
Expand Down
7 changes: 5 additions & 2 deletions drivers/goredis8/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ import (

"github.com/go-redis/redis/v8"

trm "github.com/avito-tech/go-transaction-manager/trm/v2"
"github.com/avito-tech/go-transaction-manager/trm/v2"
)

// NewDefaultFactory creates default trm.Transaction(redis.UniversalClient).
func NewDefaultFactory(db redis.UniversalClient) trm.TrFactory {
return func(ctx context.Context, trms trm.Settings) (context.Context, trm.Transaction, error) {
s, _ := trms.(Settings)
s, ok := trms.(*Settings)
if !ok {
s, _ = NewSettings(trms)
}

return NewTransaction(ctx, db, s)
}
Expand Down
15 changes: 15 additions & 0 deletions drivers/goredis8/goroutine_leak_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//go:build go1.20
// +build go1.20

package goredis8

import (
"testing"

"go.uber.org/goleak"
)

func TestMain(m *testing.M) {
// https://github.com/redis/go-redis/issues/1029
goleak.VerifyTestMain(m, goleak.IgnoreAnyFunction("github.com/go-redis/redis/v8/internal/pool.(*ConnPool).reaper"))
}
8 changes: 4 additions & 4 deletions drivers/goredis8/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import "github.com/go-redis/redis/v8"
// WithMulti sets up sql.TxOptions for the Settings.
func WithMulti(isMulti bool) Opt {
return func(s *Settings) error {
*s = s.setIsMulti(&isMulti)
s.setIsMulti(&isMulti)

return nil
}
Expand All @@ -14,7 +14,7 @@ func WithMulti(isMulti bool) Opt {
// WithWatchKeys sets WATCH keys in Transaction.
func WithWatchKeys(keys ...string) Opt {
return func(s *Settings) error {
*s = s.setWatchKeys(keys)
s.setWatchKeys(keys)

return nil
}
Expand All @@ -23,7 +23,7 @@ func WithWatchKeys(keys ...string) Opt {
// WithTxDecorator sets TxDecorator to change behavior of Transaction.
func WithTxDecorator(in TxDecorator) Opt {
return func(s *Settings) error {
*s = s.setTxDecorator(in)
s.setTxDecorator(in)

return nil
}
Expand All @@ -33,7 +33,7 @@ func WithTxDecorator(in TxDecorator) Opt {
// WARNING: Responses don't clean automatically, use WithRet only with DoWithSettings of trm.Manager.
func WithRet(in *[]redis.Cmder) Opt {
return func(s *Settings) error {
*s = s.SetReturn(in)
s.SetReturn(in)

return nil
}
Expand Down
78 changes: 54 additions & 24 deletions drivers/goredis8/settings.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package goredis8

import (
"sync"

"github.com/go-redis/redis/v8"

trm "github.com/avito-tech/go-transaction-manager/trm/v2"
"github.com/avito-tech/go-transaction-manager/trm/v2"
)

const (
Expand All @@ -20,30 +22,33 @@ type Settings struct {
isMulti *bool
watchKeys []string
txDecorator []TxDecorator
ret *[]redis.Cmder

ret *[]redis.Cmder
muRet sync.RWMutex
}

// NewSettings creates Settings.
func NewSettings(trms trm.Settings, oo ...Opt) (Settings, error) {
func NewSettings(trms trm.Settings, oo ...Opt) (*Settings, error) {
s := &Settings{
Settings: trms,
isMulti: nil,
watchKeys: nil,
txDecorator: nil,
ret: nil,
muRet: sync.RWMutex{},
}

for _, o := range oo {
if err := o(s); err != nil {
return Settings{}, err
return nil, err
}
}

return *s, nil
return s, nil
}

// MustSettings returns Settings if err is nil and panics otherwise.
func MustSettings(trms trm.Settings, oo ...Opt) Settings {
func MustSettings(trms trm.Settings, oo ...Opt) *Settings {
s, err := NewSettings(trms, oo...)
if err != nil {
panic(err)
Expand All @@ -53,8 +58,8 @@ func MustSettings(trms trm.Settings, oo ...Opt) Settings {
}

// EnrichBy fills nil properties from external Settings.
func (s Settings) EnrichBy(in trm.Settings) trm.Settings {
external, ok := in.(Settings)
func (s *Settings) EnrichBy(in trm.Settings) trm.Settings {
external, ok := in.(*Settings)
if ok {
if s.IsMultiOrNil() == nil {
s = s.SetIsMulti(external.IsMultiOrNil())
Expand All @@ -68,8 +73,8 @@ func (s Settings) EnrichBy(in trm.Settings) trm.Settings {
s = s.SetTxDecorators(external.TxDecorators()...)
}

if s.Return() == nil {
s = s.SetReturn(external.Return())
if s.ReturnPtr() == nil {
s = s.SetReturn(external.ReturnPtr())
}
}

Expand All @@ -79,7 +84,7 @@ func (s Settings) EnrichBy(in trm.Settings) trm.Settings {
}

// IsMulti - true uses redis MULTI cmd.
func (s Settings) IsMulti() bool {
func (s *Settings) IsMulti() bool {
if s.isMulti == nil {
return DefaultMulti
}
Expand All @@ -88,64 +93,89 @@ func (s Settings) IsMulti() bool {
}

// IsMultiOrNil returns IsMulti or nil.
func (s Settings) IsMultiOrNil() *bool {
func (s *Settings) IsMultiOrNil() *bool {
return s.isMulti
}

// SetIsMulti set using or not Multi for transaction, see https://redis.uptrace.dev/guide/go-redis-pipelines.html#transactions.
func (s Settings) SetIsMulti(in *bool) Settings {
func (s *Settings) SetIsMulti(in *bool) *Settings {
return s.setIsMulti(in)
}

func (s Settings) setIsMulti(in *bool) Settings {
func (s *Settings) setIsMulti(in *bool) *Settings {
s.isMulti = in

return s
}

// WatchKeys returns keys for watching.
func (s Settings) WatchKeys() []string {
func (s *Settings) WatchKeys() []string {
return s.watchKeys
}

// SetWatchKeys sets keys for watching, see https://redis.uptrace.dev/guide/go-redis-pipelines.html#watch.
func (s Settings) SetWatchKeys(in []string) Settings {
func (s *Settings) SetWatchKeys(in []string) *Settings {
return s.setWatchKeys(in)
}

func (s Settings) setWatchKeys(in []string) Settings {
func (s *Settings) setWatchKeys(in []string) *Settings {
s.watchKeys = in

return s
}

// TxDecorators returns TxDecorator decorators.
func (s Settings) TxDecorators() []TxDecorator {
func (s *Settings) TxDecorators() []TxDecorator {
return s.txDecorator
}

// SetTxDecorators sets TxDecorator decorators.
func (s Settings) SetTxDecorators(in ...TxDecorator) Settings {
func (s *Settings) SetTxDecorators(in ...TxDecorator) *Settings {
return s.setTxDecorator(in...)
}

func (s Settings) setTxDecorator(in ...TxDecorator) Settings {
func (s *Settings) setTxDecorator(in ...TxDecorator) *Settings {
s.txDecorator = in

return s
}

// Return returns []redis.Cmder from Transaction.
func (s Settings) Return() *[]redis.Cmder {
// ReturnPtr returns link to save []redis.Cmder from Transaction.
func (s *Settings) ReturnPtr() *[]redis.Cmder {
s.muRet.RLock()
defer s.muRet.RUnlock()

return s.ret
}

// Return returns []redis.Cmder from Transaction.
func (s *Settings) Return() []redis.Cmder {
res := s.ReturnPtr()
if res != nil {
return *s.ReturnPtr()
}

return nil
}

// AppendReturn append []redis.Cmder from Transaction.
func (s *Settings) AppendReturn(cmds ...redis.Cmder) {
if s.ReturnPtr() == nil {
return
}

s.muRet.Lock()
defer s.muRet.Unlock()

*s.ret = append(*s.ret, cmds...)
}

// SetReturn sets link to save []redis.Cmder from Transaction.
func (s Settings) SetReturn(in *[]redis.Cmder) Settings {
func (s *Settings) SetReturn(in *[]redis.Cmder) *Settings {
return s.setReturn(in)
}

func (s Settings) setReturn(in *[]redis.Cmder) Settings {
func (s *Settings) setReturn(in *[]redis.Cmder) *Settings {
s.ret = in

return s
Expand Down
Loading

0 comments on commit 334154e

Please sign in to comment.