Skip to content

Commit

Permalink
*: fix a data race in privilege handle Update() (pingcap#3389)
Browse files Browse the repository at this point in the history
privilege handle bind a ctx to call Update(), but it may be
called by multiple goroutines, so data race in that ctx.

change Update() to take a context parameter, avoid using the
same ctx to fix the race.
  • Loading branch information
tiancaiamao authored Jun 7, 2017
1 parent e1debd4 commit 7d634a5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 11 deletions.
6 changes: 3 additions & 3 deletions domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,8 @@ func (do *Domain) SysSessionPool() *pools.ResourcePool {
// LoadPrivilegeLoop create a goroutine loads privilege tables in a loop, it
// should be called only once in BootstrapSession.
func (do *Domain) LoadPrivilegeLoop(ctx context.Context) error {
do.privHandle = privileges.NewHandle(ctx)
err := do.privHandle.Update()
do.privHandle = privileges.NewHandle()
err := do.privHandle.Update(ctx)
if err != nil {
return errors.Trace(err)
}
Expand All @@ -450,7 +450,7 @@ func (do *Domain) LoadPrivilegeLoop(ctx context.Context) error {
case <-watchCh:
case <-time.After(duration):
}
err := do.privHandle.Update()
err := do.privHandle.Update(ctx)
if err != nil {
log.Error("load privilege fail:", errors.ErrorStack(err))
}
Expand Down
8 changes: 7 additions & 1 deletion executor/simple.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,13 @@ func (e *SimpleExec) executeFlush(s *ast.FlushStmt) error {
// TODO: A dummy implement
case ast.FlushPrivileges:
dom := sessionctx.GetDomain(e.ctx)
err := dom.PrivilegeHandle().Update()
sysSessionPool := dom.SysSessionPool()
ctx, err := sysSessionPool.Get()
if err != nil {
return errors.Trace(err)
}
defer sysSessionPool.Put(ctx)
err = dom.PrivilegeHandle().Update(ctx.(context.Context))
return errors.Trace(err)
}
return nil
Expand Down
11 changes: 4 additions & 7 deletions privilege/privileges/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,15 +566,12 @@ func appendUserPrivilegesTableRow(rows [][]types.Datum, user userRecord) [][]typ

// Handle wraps MySQLPrivilege providing thread safe access.
type Handle struct {
ctx context.Context
priv atomic.Value
}

// NewHandle returns a Handle.
func NewHandle(ctx context.Context) *Handle {
return &Handle{
ctx: ctx,
}
func NewHandle() *Handle {
return &Handle{}
}

// Get the MySQLPrivilege for read.
Expand All @@ -583,9 +580,9 @@ func (h *Handle) Get() *MySQLPrivilege {
}

// Update loads all the privilege info from kv storage.
func (h *Handle) Update() error {
func (h *Handle) Update(ctx context.Context) error {
var priv MySQLPrivilege
err := priv.LoadAll(h.ctx)
err := priv.LoadAll(ctx)
if err != nil {
return errors.Trace(err)
}
Expand Down

0 comments on commit 7d634a5

Please sign in to comment.