Skip to content

Commit

Permalink
Fix slot chain dead lock bug and deprecated resource slot chain (#379)
Browse files Browse the repository at this point in the history
  • Loading branch information
louyuting authored Jan 13, 2021
1 parent f854c93 commit e48a6ef
Show file tree
Hide file tree
Showing 23 changed files with 1 addition and 681 deletions.
6 changes: 1 addition & 5 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"sync"

"github.com/alibaba/sentinel-golang/core/base"
"github.com/alibaba/sentinel-golang/core/misc"
)

var entryOptsPool = sync.Pool{
Expand Down Expand Up @@ -142,10 +141,7 @@ func Entry(resource string, opts ...EntryOption) (*base.SentinelEntry, *base.Blo
opt(options)
}
if options.slotChain == nil {
options.slotChain = misc.GetResourceSlotChain(resource)
if options.slotChain == nil {
options.slotChain = GlobalSlotChain()
}
options.slotChain = GlobalSlotChain()
}
return entry(resource, options)
}
Expand Down
16 changes: 0 additions & 16 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ type prepareSlotMock struct {
mock.Mock
}

func (m *prepareSlotMock) Name() string {
return "mock-sentinel-prepare-slot"
}

func (m *prepareSlotMock) Order() uint32 {
return 0
}
Expand All @@ -43,10 +39,6 @@ type mockRuleCheckSlot1 struct {
mock.Mock
}

func (m *mockRuleCheckSlot1) Name() string {
return "mock-sentinel-rule-check-slot1"
}

func (m *mockRuleCheckSlot1) Order() uint32 {
return 0
}
Expand All @@ -60,10 +52,6 @@ type mockRuleCheckSlot2 struct {
mock.Mock
}

func (m *mockRuleCheckSlot2) Name() string {
return "mock-sentinel-rule-check-slot2"
}

func (m *mockRuleCheckSlot2) Order() uint32 {
return 0
}
Expand All @@ -77,10 +65,6 @@ type statisticSlotMock struct {
mock.Mock
}

func (m *statisticSlotMock) Name() string {
return "mock-sentinel-stat-check-slot"
}

func (m *statisticSlotMock) Order() uint32 {
return 0
}
Expand Down
19 changes: 0 additions & 19 deletions api/slot_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/alibaba/sentinel-golang/core/hotspot"
"github.com/alibaba/sentinel-golang/core/isolation"
"github.com/alibaba/sentinel-golang/core/log"
"github.com/alibaba/sentinel-golang/core/misc"
"github.com/alibaba/sentinel-golang/core/stat"
"github.com/alibaba/sentinel-golang/core/system"
)
Expand Down Expand Up @@ -49,21 +48,3 @@ func BuildDefaultSlotChain() *base.SlotChain {
sc.AddStatSlot(circuitbreaker.DefaultMetricStatSlot)
return sc
}

// RegisterGlobalStatPrepareSlot registers the global StatPrepareSlot for all resource
// Note: this function is not thread-safe
func RegisterGlobalStatPrepareSlot(slot base.StatPrepareSlot) {
misc.RegisterGlobalStatPrepareSlot(slot)
}

// RegisterGlobalRuleCheckSlot registers the global RuleCheckSlot for all resource
// Note: this function is not thread-safe
func RegisterGlobalRuleCheckSlot(slot base.RuleCheckSlot) {
misc.RegisterGlobalRuleCheckSlot(slot)
}

// RegisterGlobalStatSlot registers the global StatSlot for all resource
// Note: this function is not thread-safe
func RegisterGlobalStatSlot(slot base.StatSlot) {
misc.RegisterGlobalStatSlot(slot)
}
86 changes: 0 additions & 86 deletions core/base/slot_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ import (
)

type BaseSlot interface {
// Name returns it's slot name which should be global unique.
Name() string

// Order returns the sort value of the slot.
// SlotChain will sort all it's slots by ascending sort value in each bucket
// (StatPrepareSlot bucket、RuleCheckSlot bucket and StatSlot bucket)
Expand Down Expand Up @@ -77,8 +74,6 @@ type StatSlot interface {
// SlotChain hold all system slots and customized slot.
// SlotChain support plug-in slots developed by developer.
type SlotChain struct {
// RWMutex guard the slots in SlotChain and make sure the concurrency safe
sync.RWMutex
// statPres is in ascending order by StatPrepareSlot.Order() value.
statPres []StatPrepareSlot
// ruleChecks is in ascending order by RuleCheckSlot.Order() value.
Expand Down Expand Up @@ -108,7 +103,6 @@ var (

func NewSlotChain() *SlotChain {
return &SlotChain{
RWMutex: sync.RWMutex{},
statPres: make([]StatPrepareSlot, 0, 8),
ruleChecks: make([]RuleCheckSlot, 0, 8),
stats: make([]StatSlot, 0, 8),
Expand All @@ -130,31 +124,6 @@ func (sc *SlotChain) RefurbishContext(c *EntryContext) {
}
}

// ValidateStatPrepareSlotNaming checks whether the name of StatPrepareSlot exists in SlotChain.[]StatPrepareSlot
// return true the name of StatPrepareSlot doesn't exist in SlotChain.[]StatPrepareSlot
// ValidateStatPrepareSlotNaming is non-thread safe,
// In concurrency scenario, ValidateStatPrepareSlotNaming must be guarded by SlotChain.RWMutex#RLock
func ValidateStatPrepareSlotNaming(sc *SlotChain, s StatPrepareSlot) bool {
isValid := true
f := func(slot StatPrepareSlot) {
if slot.Name() == s.Name() {
isValid = false
}
}
sc.RangeStatPrepareSlot(f)

return isValid
}

// RangeStatPrepareSlot iterates the SlotChain.[]StatPrepareSlot and call f function for each StatPrepareSlot
// RangeStatPrepareSlot is non-thread safe,
// In concurrency scenario, RangeStatPrepareSlot must be guarded by SlotChain.RWMutex#RLock
func (sc *SlotChain) RangeStatPrepareSlot(f func(slot StatPrepareSlot)) {
for _, slot := range sc.statPres {
f(slot)
}
}

// AddStatPrepareSlot adds the StatPrepareSlot slot to the StatPrepareSlot list of the SlotChain.
// All StatPrepareSlot in the list will be sorted according to StatPrepareSlot.Order() in ascending order.
// AddStatPrepareSlot is non-thread safe,
Expand All @@ -166,31 +135,6 @@ func (sc *SlotChain) AddStatPrepareSlot(s StatPrepareSlot) {
})
}

// ValidateRuleCheckSlotNaming checks whether the name of RuleCheckSlot exists in SlotChain.[]RuleCheckSlot
// return true the name of RuleCheckSlot doesn't exist in SlotChain.[]RuleCheckSlot
// ValidateRuleCheckSlotNaming is non-thread safe,
// In concurrency scenario, ValidateRuleCheckSlotNaming must be guarded by SlotChain.RWMutex#RLock
func ValidateRuleCheckSlotNaming(sc *SlotChain, s RuleCheckSlot) bool {
isValid := true
f := func(slot RuleCheckSlot) {
if slot.Name() == s.Name() {
isValid = false
}
}
sc.RangeRuleCheckSlot(f)

return isValid
}

// RangeRuleCheckSlot iterates the SlotChain.[]RuleCheckSlot and call f function for each RuleCheckSlot
// RangeRuleCheckSlot is non-thread safe,
// In concurrency scenario, RangeRuleCheckSlot must be guarded by SlotChain.RWMutex#RLock
func (sc *SlotChain) RangeRuleCheckSlot(f func(slot RuleCheckSlot)) {
for _, slot := range sc.ruleChecks {
f(slot)
}
}

// AddRuleCheckSlot adds the RuleCheckSlot to the RuleCheckSlot list of the SlotChain.
// All RuleCheckSlot in the list will be sorted according to RuleCheckSlot.Order() in ascending order.
// AddRuleCheckSlot is non-thread safe,
Expand All @@ -202,31 +146,6 @@ func (sc *SlotChain) AddRuleCheckSlot(s RuleCheckSlot) {
})
}

// ValidateStatSlotNaming checks whether the name of StatSlot exists in SlotChain.[]StatSlot
// return true the name of StatSlot doesn't exist in SlotChain.[]StatSlot
// ValidateStatSlotNaming is non-thread safe,
// In concurrency scenario, ValidateStatSlotNaming must be guarded by SlotChain.RWMutex#RLock
func ValidateStatSlotNaming(sc *SlotChain, s StatSlot) bool {
isValid := true
f := func(slot StatSlot) {
if slot.Name() == s.Name() {
isValid = false
}
}
sc.RangeStatSlot(f)

return isValid
}

// RangeStatSlot iterates the SlotChain.[]StatSlot and call f function for each StatSlot
// RangeStatSlot is non-thread safe,
// In concurrency scenario, RangeStatSlot must be guarded by SlotChain.RWMutex#RLock
func (sc *SlotChain) RangeStatSlot(f func(slot StatSlot)) {
for _, slot := range sc.stats {
f(slot)
}
}

// AddStatSlot adds the StatSlot to the StatSlot list of the SlotChain.
// All StatSlot in the list will be sorted according to StatSlot.Order() in ascending order.
// AddStatSlot is non-thread safe,
Expand All @@ -241,11 +160,9 @@ func (sc *SlotChain) AddStatSlot(s StatSlot) {
// The entrance of slot chain
// Return the TokenResult and nil if internal panic.
func (sc *SlotChain) Entry(ctx *EntryContext) *TokenResult {
sc.RLock()
// This should not happen, unless there are errors existing in Sentinel internal.
// If happened, need to add TokenResult in EntryContext
defer func() {
sc.RUnlock()
if err := recover(); err != nil {
logging.Error(errors.Errorf("%+v", err), "Sentinel internal panic in SlotChain.Entry()")
ctx.SetError(errors.Errorf("%+v", err))
Expand Down Expand Up @@ -311,9 +228,6 @@ func (sc *SlotChain) exit(ctx *EntryContext) {
if ctx.IsBlocked() {
return
}

sc.RLock()
defer sc.RUnlock()
for _, s := range sc.stats {
s.OnCompleted(ctx)
}
Expand Down
Loading

0 comments on commit e48a6ef

Please sign in to comment.