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

log filter generates race alarm #2968

Closed
chadlwm opened this issue Aug 28, 2023 · 4 comments · May be fixed by #2969
Closed

log filter generates race alarm #2968

chadlwm opened this issue Aug 28, 2023 · 4 comments · May be fixed by #2969
Labels
bug Something isn't working

Comments

@chadlwm
Copy link

chadlwm commented Aug 28, 2023

What happened:

使用log filter时产生race告警

// Log Print log by level and keyvals.
func (f *Filter) Log(level Level, keyvals ...interface{}) error {
	if level < f.level {
		return nil
	}
	// prefixkv contains the slice of arguments defined as prefixes during the log initialization
	var prefixkv []interface{}
	l, ok := f.logger.(*logger)
	if ok {
		l.ctx = f.ctx  // 并发写时,会触发竞争
	}
     
       ...
}

What you expected to happen:

log filter中对ctx的赋值,未添加所,多个go routine访问时会产生竞争

How to reproduce it (as minimally and precisely as possible):

在main.go
logger := log.NewFilter(logger, log.FilterLevel(log.ParseLevel("info")))

在多个server中使用logger:
log.NewHelper(logger)

build with -race
mkdir -p app/ && go build -race --trimpath -v -ldflags "-X main.Version=$(VERSION)" -o ./app/ ./...

启动
./app/xxxxx -conf configs

日志warning:

==================
WARNING: DATA RACE
Write at 0x00c00012a6b0 by goroutine 30:
github.com/go-kratos/kratos/v2/log.(*Filter).Log()
github.com/go-kratos/kratos/v2@v2.6.3/log/filter.go:74 +0x10d
github.com/go-kratos/kratos/v2/log.Infof()
github.com/go-kratos/kratos/v2@v2.6.3/log/global.go:73 +0x189
github.com/go-kratos/kratos/v2/transport/grpc.(*Server).Start()
github.com/go-kratos/kratos/v2@v2.6.3/transport/grpc/server.go:205 +0x104
github.com/go-kratos/kratos/v2.(*App).Run.func2()
github.com/go-kratos/kratos/v2@v2.6.3/app.go:112 +0x81
golang.org/x/sync/errgroup.(*Group).Go.func1()
golang.org/x/sync@v0.3.0/errgroup/errgroup.go:75 +0x82

Previous write at 0x00c00012a6b0 by goroutine 35:
github.com/go-kratos/kratos/v2/log.(*Filter).Log()
github.com/go-kratos/kratos/v2@v2.6.3/log/filter.go:74 +0x10d
github.com/go-kratos/kratos/v2/log.(*logger).Log()
github.com/go-kratos/kratos/v2@v2.6.3/log/log.go:30 +0x242
github.com/go-kratos/kratos/v2/log.(*Helper).Infof()
github.com/go-kratos/kratos/v2@v2.6.3/log/helper.go:96 +0x192
linkage-signal-service/internal/server.(*CS104SingleServer).Start.func1()
xxxxxx/internal/server/cs104_single.go:72 +0x14b

Goroutine 30 (running) created at:
golang.org/x/sync/errgroup.(*Group).Go()
golang.org/x/sync@v0.3.0/errgroup/errgroup.go:72 +0x12e
github.com/go-kratos/kratos/v2.(*App).Run()
github.com/go-kratos/kratos/v2@v2.6.3/app.go:110 +0x319
main.main()
xxxxxx/cmd/xxxxxx/main.go:94 +0xafa

Goroutine 35 (running) created at:
xxxxxx/internal/server.(*CS104SingleServer).Start()
xxxxxx/internal/server/cs104_single.go:68 +0x8e
github.com/go-kratos/kratos/v2.(*App).Run.func2()
github.com/go-kratos/kratos/v2@v2.6.3/app.go:112 +0x81
golang.org/x/sync/errgroup.(*Group).Go.func1()
golang.org/x/sync@v0.3.0/errgroup/errgroup.go:75 +0x82

Anything else we need to know?:

Environment:

  • Kratos version (use kratos -v): kratos version v2.6.3
  • Go version (use go version): go version go1.20.3 linux/amd64
  • OS (e.g: cat /etc/os-release): ubuntu 16.04.7 LTS (Xenial Xerus)
  • Others:
@chadlwm chadlwm added the bug Something isn't working label Aug 28, 2023
@kratos-ci-bot kratos-ci-bot changed the title log filter 产生race告警 log filter generates race alarm Aug 28, 2023
@kvii
Copy link
Contributor

kvii commented Sep 7, 2023

A minimal reproduce:

package main

import (
	"io"
	"sync"

	"github.com/go-kratos/kratos/v2/log"
)

func main() {
	l := log.With(log.NewStdLogger(io.Discard))
	l = log.NewFilter(l)

	var wg sync.WaitGroup
	for i := 0; i < 3; i++ {
		wg.Add(1)
		go func() { defer wg.Done(); l.Log(log.LevelInfo, "a") }()
	}
	wg.Wait()
}

output:

$ go run -race .

==================
WARNING: DATA RACE
Write at 0x00c000108070 by goroutine 8:
  github.com/go-kratos/kratos/v2/log.(*Filter).Log()
      /Users/kvii/go/pkg/mod/github.com/go-kratos/kratos/v2@v2.7.0/log/filter.go:74 +0xcc
  main.main.func1()
      /Users/kvii/Documents/workspace/qs/go/playground/main.go:17 +0xd8

Previous write at 0x00c000108070 by goroutine 6:
  github.com/go-kratos/kratos/v2/log.(*Filter).Log()
      /Users/kvii/go/pkg/mod/github.com/go-kratos/kratos/v2@v2.7.0/log/filter.go:74 +0xcc
  main.main.func1()
      /Users/kvii/Documents/workspace/qs/go/playground/main.go:17 +0xd8

Goroutine 8 (running) created at:
  main.main()
      /Users/kvii/Documents/workspace/qs/go/playground/main.go:17 +0x144

Goroutine 6 (running) created at:
  main.main()
      /Users/kvii/Documents/workspace/qs/go/playground/main.go:17 +0x144
==================
Found 1 data race(s)
exit status 66

Copy link

dosubot bot commented Dec 7, 2023

Hi, @chadlwm,

I'm helping the Kratos team manage their backlog and am marking this issue as stale. The issue was raised by you regarding a race condition in the log filter of Kratos v2.6.3. User @kvii provided a minimal reproduction of the issue, and you acknowledged it with a thumbs-up reaction. As of now, the issue remains unresolved.

Could you please confirm if this issue is still relevant to the latest version of the Kratos repository? If it is, kindly let the Kratos team know by commenting on the issue. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your understanding and cooperation.

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Dec 7, 2023
@kvii
Copy link
Contributor

kvii commented Dec 8, 2023

Bug has fixed at #3069. @chadlwm You can try it with my previous comment, then close this issue.

@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Dec 8, 2023
Copy link

dosubot bot commented Dec 8, 2023

@shenqidebaozi, could you please assist with the issue raised by @chadlwm regarding the race condition in the log filter of Kratos v2.6.3? User @kvii mentioned that the bug has been fixed at #3069, and @chadlwm is advised to try it with the previous comment and then close the issue. Thank you!

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Mar 9, 2024
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 16, 2024
@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants