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

fix(discovery):Fix the protected data race #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pretty66
Copy link

After the program starts, one of the interfaces is requested, and the warning level data contention will be triggered after 60 seconds

func (d *Discovery) exitProtect() {
    // exist protect mode after two renew cycle, data race after 60s
    time.Sleep(time.Second * 60)
    d.protected = false
}
INFO 12/25-11:51:59.456 http-access-log stack=<nil> timeout_quota=39.999948634 ip=127.0.0.1 user=no_user params= msg=0 ts=0.000798093 traceid= ret=0 method=GET path=/discovery/fetch/all err= 
==================
WARNING: DATA RACE
Write at 0x00c00018c8a8 by goroutine 26:
  github.com/bilibili/discovery/discovery.(*Discovery).exitProtect()
      /Users/work/discovery/discovery/discovery.go:41 +0x51

Previous read at 0x00c00018c8a8 by goroutine 33:
  github.com/bilibili/discovery/discovery.(*Discovery).Protected()
      /Users/work/discovery/discovery/syncup.go:24 +0x59
  github.com/bilibili/discovery/http.initProtect()
      /Users/work/discovery/http/http.go:50 +0x2f
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Context).Next()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/context.go:84 +0xa6
  github.com/go-kratos/kratos/pkg/net/http/blademaster.Logger.func1()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/logger.go:26 +0x174
  github.com/go-kratos/kratos/pkg/net/http/blademaster.HandlerFunc.ServeHTTP()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:72 +0x3d
  github.com/go-kratos/kratos/pkg/net/http/blademaster.Handler.ServeHTTP-fm()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:64 +0x50
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Context).Next()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/context.go:84 +0xa6
  github.com/go-kratos/kratos/pkg/net/http/blademaster.Trace.func1()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/trace.go:38 +0x8a3
  github.com/go-kratos/kratos/pkg/net/http/blademaster.HandlerFunc.ServeHTTP()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:72 +0x3d
  github.com/go-kratos/kratos/pkg/net/http/blademaster.Handler.ServeHTTP-fm()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:64 +0x50
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Context).Next()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/context.go:84 +0xa6
  github.com/go-kratos/kratos/pkg/net/http/blademaster.Recovery.func1()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/recovery.go:30 +0x6c
  github.com/go-kratos/kratos/pkg/net/http/blademaster.HandlerFunc.ServeHTTP()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:72 +0x3d
  github.com/go-kratos/kratos/pkg/net/http/blademaster.Handler.ServeHTTP-fm()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:64 +0x50
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Context).Next()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/context.go:84 +0xa6
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Engine).handleContext()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:327 +0x5f8
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Engine).ServeHTTP()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:490 +0x252
  net/http.serverHandler.ServeHTTP()
      /usr/local/Cellar/go/1.14.5/libexec/src/net/http/server.go:2836 +0xce
  net/http.(*conn).serve()
      /usr/local/Cellar/go/1.14.5/libexec/src/net/http/server.go:1924 +0x837

Goroutine 26 (running) created at:
  github.com/bilibili/discovery/discovery.New()
      /Users/work/discovery/discovery/discovery.go:34 +0x21d
  main.main()
      /Users/work/discovery/cmd/discovery/main.go:23 +0x14d

Goroutine 33 (running) created at:
  net/http.(*Server).Serve()
      /usr/local/Cellar/go/1.14.5/libexec/src/net/http/server.go:2962 +0x5b6
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Engine).RunServer()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:462 +0xc1
  github.com/go-kratos/kratos/pkg/net/http/blademaster.(*Engine).Start.func1()
      /Users/work/gopath/pkg/mod/github.com/go-kratos/kratos@v0.6.0/pkg/net/http/blademaster/server.go:103 +0x77
==================

@@ -45,7 +48,9 @@ func (d *Discovery) syncUp() {
continue
}
// sync success from other node,exit protected mode
d.lock.Lock()
d.protected = false

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious about shouldn't exiting protected mode after all instances finish register?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants