Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions openfeature/event_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,16 @@ func (e *eventExecutor) GetClientRegistry(client string) scopedCallback {
// emitOnRegistration fulfils the spec requirement to fire events if the
// event type and the state of the associated provider are compatible.
func (e *eventExecutor) emitOnRegistration(domain string, providerReference providerReference, eventType EventType, callback EventCallback) {
state, ok := e.loadState(domain)
if !ok {
return
var state State
// state-managing providers own their state; read directly
if smp, ok := providerReference.featureProvider.(StateManagingProvider); ok {
state = smp.State()
} else {
Comment on lines +155 to +157

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since providerReference now includes the delegateManagesState flag, we can use it here to avoid the type assertion check, improving clarity and consistency with the new field.

Suggested change
if smp, ok := providerReference.featureProvider.(StateManagingProvider); ok {
state = smp.State()
} else {
if providerReference.delegateManagesState {
state = providerReference.featureProvider.(StateManagingProvider).State()
} else {

var ok bool
state, ok = e.loadState(domain)
if !ok {
return
}
}

var message string
Expand Down Expand Up @@ -185,6 +192,20 @@ func (e *eventExecutor) loadState(domain string) (State, bool) {
}

func (e *eventExecutor) State(domain string) State {
e.mu.Lock()
defer e.mu.Unlock()

// find the provider reference for this domain
ref, ok := e.namedProviderReference[domain]
if !ok {
ref = e.defaultProviderReference
}

// state-managing providers own their state; read directly
if smp, ok := ref.featureProvider.(StateManagingProvider); ok {
return smp.State()
}
Comment on lines +195 to +207

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Holding the executor's mutex while calling smp.State() is risky as it involves calling into external provider code while holding a lock. This can lead to performance bottlenecks or potential deadlocks if the provider's State() method blocks or attempts to call back into the SDK. Since we only need the lock to retrieve the provider reference, we can release it early. Additionally, we can leverage the delegateManagesState flag.

	e.mu.Lock()
	// find the provider reference for this domain
	ref, ok := e.namedProviderReference[domain]
	if !ok {
		ref = e.defaultProviderReference
	}
	e.mu.Unlock()

	// state-managing providers own their state; read directly
	if ref.delegateManagesState {
		return ref.featureProvider.(StateManagingProvider).State()
	}


state, _ := e.loadState(domain)
return state
}
Expand Down Expand Up @@ -297,6 +318,8 @@ func (e *eventExecutor) triggerEvent(event Event, handler FeatureProvider) {
e.mu.Lock()
defer e.mu.Unlock()

_, delegateManagesState := handler.(StateManagingProvider)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This type assertion is redundant because the providerReference already caches this information in the delegateManagesState field. You can remove this line and use the flag from the references in the subsequent logic (e.g., reference.delegateManagesState and e.defaultProviderReference.delegateManagesState).


// first run API handlers
for _, c := range e.apiRegistry[event.EventType] {
e.executeHandler(*c, event)
Expand All @@ -308,7 +331,10 @@ func (e *eventExecutor) triggerEvent(event Event, handler FeatureProvider) {
continue
}

e.states.Store(domain, stateFromEvent(event))
// state-managing providers own their state; skip SDK-side writes
if !delegateManagesState {
e.states.Store(domain, stateFromEvent(event))
}
for _, c := range e.scopedRegistry[domain].callbacks[event.EventType] {
e.executeHandler(*c, event)
}
Expand All @@ -319,7 +345,10 @@ func (e *eventExecutor) triggerEvent(event Event, handler FeatureProvider) {
}

// handling the default provider
e.states.Store(defaultDomain, stateFromEvent(event))
// state-managing providers own their state; skip SDK-side writes
if !delegateManagesState {
e.states.Store(defaultDomain, stateFromEvent(event))
}
// invoke default provider bound (no provider associated) handlers by filtering
for domain, registry := range e.scopedRegistry {
if _, ok := e.namedProviderReference[domain]; ok {
Expand Down
8 changes: 7 additions & 1 deletion openfeature/openfeature_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,16 @@ func (api *evaluationAPI) setNamedProviderWithContext(ctx context.Context, clien
func (api *evaluationAPI) initNew(ctx context.Context, clientName string, newProvider FeatureProvider) <-chan error {
errCh := make(chan error, 1)

_, delegateManagesState := newProvider.(StateManagingProvider)

// Initialize new provider async. The caller may wait on the channel.
go func(executor *eventExecutor, evalCtx EvaluationContext, ctx context.Context, provider FeatureProvider, clientName string) {
event, err := initializerWithContext(ctx, provider, evalCtx)
executor.triggerEvent(event, provider)

// State-managing providers emit their own events; skip SDK-side emission.
if !delegateManagesState {
executor.triggerEvent(event, provider)
}

if err != nil {
if clientName == "" {
Expand Down
22 changes: 22 additions & 0 deletions openfeature/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,28 @@ func (s *NoopStateHandler) Shutdown() {
// NOOP
}

// StateManagingProvider is a provider that manages its own state. The SDK reads
// state from the provider rather than maintaining shadow state. Implementations
// MUST ensure that State() is safe for concurrent access and that state
// transitions and associated event emissions are atomic from the perspective of
// external observers.
//
// Legacy providers that do not implement this interface continue to have their
// state managed by the SDK (deprecated behavior; to be removed in the next
// major version).
type StateManagingProvider interface {
FeatureProvider
StateHandler
EventHandler

// State returns the current provider state. Must reflect NotReadyState
// before Init is called and after Shutdown completes. Must reflect
// ReadyState if Init returns nil.
//
// This method must be safe for concurrent access.
State() State
}

// Eventing

// EventHandler is the eventing contract enforced for FeatureProvider
Expand Down
10 changes: 7 additions & 3 deletions openfeature/reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (

// newProviderRef creates a new providerReference instance that wraps around a FeatureProvider implementation
func newProviderRef(provider FeatureProvider) providerReference {
_, managesState := provider.(StateManagingProvider)
return providerReference{
featureProvider: provider,
kind: reflect.TypeOf(provider).Kind(),
shutdownSemaphore: make(chan any, 1),
featureProvider: provider,
kind: reflect.TypeOf(provider).Kind(),
shutdownSemaphore: make(chan any, 1),
delegateManagesState: managesState,
}
}

Expand All @@ -19,6 +21,8 @@ type providerReference struct {
featureProvider FeatureProvider
kind reflect.Kind
shutdownSemaphore chan any
// delegateManagesState is true when the provider implements StateManagingProvider
delegateManagesState bool
}

func (pr providerReference) equals(other providerReference) bool {
Expand Down
Loading