[Race/Multi-threading] Plugin status listeners receive stale plugin status

[Pushpalanka]

Short description

(Not a classic data race)
Manager.UpdatePluginStatus() delivers status notifications to listeners out-of-order when concurrent updates occur. Once a listener receives a stale notification, it never receives a correction as plugins seems to send updates only on state transitions.

Environment:
OPA version: v1.7.1
Integration: Applications using OPA as library and using plugins.Manager.RegisterPluginStatusListener()
Example: Skipper OPA integration

Impact:
External systems receive incorrect plugin state
State never corrects itself (requires restart)
Affects OPA instance health checks

Steps To Reproduce

1. Register a status listener via manager.RegisterPluginStatusListener()
2. Configure bundle plugin with discovery enabled (in a higher concurrency. ~50 bundles getting activated in parallel execution)
3. Start OPA - bundles download and activate: NOT_READY → OK
4. Observe listener receives: {OK} then {NOT_READY} (stale snapshot from earlier)
5. Listener's view is now incorrect and never recovers

Expected behavior

Expected: Listeners receive status snapshots in the order UpdatePluginStatus() was called
Actual: Listeners can receive snapshots out-of-order due to goroutine scheduling

Additional context

With locally added more logging, it produced a log as below,

log placement in code

func (m *Manager) UpdatePluginStatus(pluginName string, status *Status) {
	m.mtx.Lock()
	defer m.mtx.Unlock()

	m.pluginStatus[pluginName] = status
	toNotify := make(map[string]StatusListener, len(m.pluginStatusListeners))
	maps.Copy(toNotify, m.pluginStatusListeners)
	statuses := m.copyPluginStatus()

	// Release lock before calling listeners, but use a goroutine to avoid
	// blocking the caller while still ensuring sequential notification
	m.mtx.Unlock()
	
	for _, l := range toNotify {
		**m.logger.Info("UpdatePluginStatus: plugin status listener %+v  status manager %+v", statuses, m.pluginStatus)**
		l(statuses)
	}

	m.mtx.Lock() // Re-acquire for defer
}

Produced log at high concurrency

[APP]time="2025-11-02T02:14:04+01:00" level=info msg="UpdatePluginStatus: plugin status listener map[bundle:{**NOT_READY** \"\"} decision_logs:{OK \"\"} discovery:{OK \"\"} envoy_ext_authz_grpc:{OK \"\"} status:{OK \"\"}] status manager map[bundle:{**OK** \"\"} decision_logs:{OK \"\"} discovery:{OK \"\"} envoy_ext_authz_grpc:{OK \"\"} status:{OK \"\"}]" bundle-name=bundles/discovery6.tar.gz

logs.txt
As for the logs no status update is triggered again even-though the bundle plugin is on OK state

Time	Routine A	Routine B
T1	UpdatePluginStatus("bundle", NOT_READY)
T2	Lock → Update → Create snapshot S1 {NOT_READY} → Unlock
T3	Preempted before calling listeners
T4		UpdatePluginStatus("bundle", OK)
T5		Lock → Update → Create snapshot S2 {OK} → Unlock
T6		Call listeners with Snapshot S2
T7	Resumes - Call listeners with old snapshot S1 ❌

From there onward No status updates are sent out when no change happens to the bundles. checkPluginReadiness() doesn't UpdateState to OK as plugin is already in OK ready state.

PS: shared a more detailed version in my blog
Appreciate your input/support to handle or fix the issue.

[sspaink]

@Pushpalanka great job on the thorough investigation! I would assume moving the call to the listeners inside of the lock could fix this problem. Probably was put outside of the lock to improve performance but reporting the status out of sync isn't worth it. Would you be interested in working on this further and try out that change and see if we can add a unit test if it does fix the problem?

[Pushpalanka]

@Pushpalanka great job on the thorough investigation! I would assume moving the call to the listeners inside of the lock could fix this problem. Probably was put outside of the lock to improve performance but reporting the status out of sync isn't worth it. Would you be interested in working on this further and try out that change and see if we can add a unit test if it does fix the problem?

Thanks @sspaink
I actually tried moving the listener calls inside the lock during my debugging. It immediately caused a deadlock and the server failed to start.
This happens because the listener logic often needs to query the Manager to get the latest state or context.

So that approach creates an open risk for a deadlock when the unknown listener code is executed within this lock.
I am thinking we need a better option(a buffered channel or a queue mehanism).
I am interested in working on the fix. Appreciate your thoughts on the approach!

[sspaink]

Darn 😅 , well thank you for trying it! I think using a buffered channel sounds like a great idea. The status plugin uses a buffered channel as well: https://github.com/open-policy-agent/opa/blob/main/v1/plugins/status/plugin.go#L367-L376

[stale]

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.