mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-04-30 04:23:01 +00:00
fix(plugin_state): lock _error_info accesses and store defensive copies
Three verified issues: - set_error_info wrote _error_info without holding _lock and stored the caller's dict by reference, allowing races and post-write mutation - set_state_with_error stored error_info by reference (lock was already held) - get_error_info read _error_info without _lock and returned the live reference, letting callers mutate the stored snapshot Implicit fourth fix: set_state also wrote _error_info without _lock; locking get_error_info while leaving that writer unguarded would have created a new race, so set_state is now wrapped in _lock too for consistency. Changes: - set_state: wrap entire body in self._lock (covers _states, _state_history, and _error_info writes atomically; ERROR-path _error_info value was already a fresh dict literal so no copy needed) - set_error_info: acquire self._lock + store dict(error_info) shallow copy - set_state_with_error: store dict(error_info) shallow copy (lock already held) - get_error_info: acquire self._lock + return dict(info) copy or None All stored values are flat dicts of strings/floats/bools, so shallow copy is sufficient — deepcopy is not needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -50,44 +50,44 @@ class PluginStateManager:
|
||||
) -> None:
|
||||
"""
|
||||
Set plugin state and record transition.
|
||||
|
||||
|
||||
Args:
|
||||
plugin_id: Plugin identifier
|
||||
state: New state
|
||||
error: Optional error if transitioning to ERROR state
|
||||
"""
|
||||
old_state = self._states.get(plugin_id, PluginState.UNLOADED)
|
||||
self._states[plugin_id] = state
|
||||
|
||||
# Record state transition
|
||||
if plugin_id not in self._state_history:
|
||||
self._state_history[plugin_id] = []
|
||||
|
||||
transition = {
|
||||
'timestamp': datetime.now(),
|
||||
'from': old_state.value,
|
||||
'to': state.value,
|
||||
'error': str(error) if error else None
|
||||
}
|
||||
self._state_history[plugin_id].append(transition)
|
||||
|
||||
# Store error info if transitioning to ERROR state
|
||||
if state == PluginState.ERROR and error:
|
||||
self._error_info[plugin_id] = {
|
||||
'error': str(error),
|
||||
'error_type': type(error).__name__,
|
||||
'timestamp': datetime.now()
|
||||
with self._lock:
|
||||
old_state = self._states.get(plugin_id, PluginState.UNLOADED)
|
||||
self._states[plugin_id] = state
|
||||
|
||||
if plugin_id not in self._state_history:
|
||||
self._state_history[plugin_id] = []
|
||||
|
||||
transition = {
|
||||
'timestamp': datetime.now(),
|
||||
'from': old_state.value,
|
||||
'to': state.value,
|
||||
'error': str(error) if error else None
|
||||
}
|
||||
elif state != PluginState.ERROR:
|
||||
# Clear error info when leaving ERROR state
|
||||
self._error_info.pop(plugin_id, None)
|
||||
|
||||
self.logger.debug(
|
||||
"Plugin %s state transition: %s → %s",
|
||||
plugin_id,
|
||||
old_state.value,
|
||||
state.value
|
||||
)
|
||||
self._state_history[plugin_id].append(transition)
|
||||
|
||||
# Store error info if transitioning to ERROR state
|
||||
if state == PluginState.ERROR and error:
|
||||
self._error_info[plugin_id] = {
|
||||
'error': str(error),
|
||||
'error_type': type(error).__name__,
|
||||
'timestamp': datetime.now()
|
||||
}
|
||||
elif state != PluginState.ERROR:
|
||||
# Clear error info when leaving ERROR state
|
||||
self._error_info.pop(plugin_id, None)
|
||||
|
||||
self.logger.debug(
|
||||
"Plugin %s state transition: %s → %s",
|
||||
plugin_id,
|
||||
old_state.value,
|
||||
state.value
|
||||
)
|
||||
|
||||
def get_state(self, plugin_id: str) -> PluginState:
|
||||
"""
|
||||
@@ -149,7 +149,8 @@ class PluginStateManager:
|
||||
plugin_id: Plugin identifier
|
||||
error_info: Arbitrary dict describing the error
|
||||
"""
|
||||
self._error_info[plugin_id] = error_info
|
||||
with self._lock:
|
||||
self._error_info[plugin_id] = dict(error_info)
|
||||
|
||||
def set_state_with_error(
|
||||
self,
|
||||
@@ -187,7 +188,7 @@ class PluginStateManager:
|
||||
'error': str(error) if error else None,
|
||||
})
|
||||
|
||||
self._error_info[plugin_id] = error_info
|
||||
self._error_info[plugin_id] = dict(error_info)
|
||||
|
||||
self.logger.debug(
|
||||
"Plugin %s state transition: %s → %s (recoverable error stored)",
|
||||
@@ -201,15 +202,18 @@ class PluginStateManager:
|
||||
Get error information for a plugin.
|
||||
|
||||
Returns the stored error dict whether the plugin is in ERROR state or
|
||||
still ENABLED after a recoverable failure.
|
||||
still ENABLED after a recoverable failure. Returns a shallow copy so
|
||||
callers cannot mutate the stored snapshot.
|
||||
|
||||
Args:
|
||||
plugin_id: Plugin identifier
|
||||
|
||||
Returns:
|
||||
Error information dict or None
|
||||
Copy of the error information dict, or None
|
||||
"""
|
||||
return self._error_info.get(plugin_id)
|
||||
with self._lock:
|
||||
info = self._error_info.get(plugin_id)
|
||||
return dict(info) if info is not None else None
|
||||
|
||||
def record_update(self, plugin_id: str) -> None:
|
||||
"""Record that plugin update() was called."""
|
||||
|
||||
Reference in New Issue
Block a user