Fix crash on exit from device cleanup during interpreter shutdown
`solaar show` (and any exit path) printed a cascade of "I/O operation on closed file" / "sys.meta_path is None" logging errors at the bottom. Two compounding causes: - Device.close() was not idempotent. The CLI closes devices explicitly, then __del__ calls close() again during interpreter shutdown, re-running every device cleanup a second time. Guard with a _closed flag (not self.handle, which is legitimately None for receiver-paired devices). - rgb_power.cleanup() read device.settings — the lazy property — which forces a first-time settings build. Running that build during shutdown fails because gettext can no longer import (sys.meta_path is None), raising inside a __del__ handler. Read the already-built settings (or the persister) via a new _rgb_control_off() helper that never triggers the build. Either guard alone stops the crash; both are correct hardening for a __del__-invoked path.
This commit is contained in:
parent
5e4ae7261e
commit
012d7fd04d
|
|
@ -172,6 +172,7 @@ class Device:
|
|||
self.present = True # used for devices that are integral with their receiver but that separately be disconnected
|
||||
|
||||
self._feature_settings_checked = False
|
||||
self._closed = False
|
||||
self._gestures_lock = threading.Lock()
|
||||
self._settings_lock = threading.Lock()
|
||||
self._persister_lock = threading.Lock()
|
||||
|
|
@ -1037,6 +1038,12 @@ class Device:
|
|||
pass
|
||||
|
||||
def close(self):
|
||||
# Idempotent: __del__ re-invokes close() at interpreter shutdown, where
|
||||
# re-running cleanups can't safely build settings. Flag, not self.handle
|
||||
# (None for receiver-paired devices that borrow the receiver's handle).
|
||||
if getattr(self, "_closed", False):
|
||||
return None
|
||||
self._closed = True
|
||||
# Run device.cleanups before clearing self.handle — cleanup callbacks
|
||||
# typically need to issue final feature_request() writes (e.g. release
|
||||
# SW control, restore device-side state) and feature_request() relies
|
||||
|
|
|
|||
|
|
@ -316,6 +316,21 @@ def stop(device):
|
|||
mgr.stop()
|
||||
|
||||
|
||||
def _rgb_control_off(device):
|
||||
# Reads already-built settings/persister, never device.settings (the lazy
|
||||
# property): cleanup() runs from __del__, where a settings build fails at
|
||||
# shutdown. Unknown state ⇒ False (proceed).
|
||||
for s in getattr(device, "_settings", None) or []:
|
||||
if s.name == "rgb_control":
|
||||
return not s._value
|
||||
persister = getattr(device, "persister", None)
|
||||
if persister is not None:
|
||||
value = persister.get("rgb_control")
|
||||
if value is not None:
|
||||
return value in (False, 0)
|
||||
return False
|
||||
|
||||
|
||||
def cleanup(device):
|
||||
"""device.cleanups handler — restore firmware control on device close.
|
||||
|
||||
|
|
@ -330,7 +345,7 @@ def cleanup(device):
|
|||
animation would visibly contradict the user's "leave my lighting alone".
|
||||
"""
|
||||
stop(device)
|
||||
if any(s.name == "rgb_control" and not s._value for s in getattr(device, "settings", []) or []):
|
||||
if _rgb_control_off(device):
|
||||
return
|
||||
try:
|
||||
device.feature_request(SupportedFeature.RGB_EFFECTS, 0x50, SW_RELEASE)
|
||||
|
|
|
|||
|
|
@ -423,3 +423,20 @@ def test_device_battery(device_info, responses, protocol, expected_battery, chan
|
|||
assert test_device.battery() == expected_battery
|
||||
test_device.read_battery()
|
||||
spy_changed.assert_called_with(**changed)
|
||||
|
||||
|
||||
def test_close_runs_cleanups_once():
|
||||
"""close() is idempotent: __del__ calls it again at interpreter shutdown,
|
||||
and cleanups must not run a second time (a settings build there fails)."""
|
||||
handle = 0x1
|
||||
test_device = device.Device(
|
||||
LowLevelInterfaceFake(fake_hidpp.r_empty), None, None, None, handle=handle, device_info=di_CCCC
|
||||
)
|
||||
calls = []
|
||||
test_device.cleanups = [lambda d: calls.append(d)]
|
||||
|
||||
test_device.close()
|
||||
test_device.close() # simulates the __del__ re-entry
|
||||
test_device.__del__()
|
||||
|
||||
assert calls == [test_device]
|
||||
|
|
|
|||
|
|
@ -672,3 +672,42 @@ def test_perkey_write_key_value_skipped_when_zone_is_animation(monkeypatch):
|
|||
s.write_key_value(7, 0xFF0000)
|
||||
|
||||
s._send_zone_color.assert_not_called()
|
||||
|
||||
|
||||
class _SettingsPropertyExplodes:
|
||||
"""A device whose `.settings` property raises — mirrors the interpreter
|
||||
-shutdown case where forcing a lazy settings build fails. cleanup() must
|
||||
never touch it."""
|
||||
|
||||
def __init__(self, settings_list=None, persister=None):
|
||||
self._settings = settings_list
|
||||
self.persister = persister
|
||||
|
||||
@property
|
||||
def settings(self):
|
||||
raise AssertionError("cleanup must not access the settings property")
|
||||
|
||||
|
||||
def _rgb_control_setting(value):
|
||||
class _S:
|
||||
name = "rgb_control"
|
||||
|
||||
s = _S()
|
||||
s._value = value
|
||||
return s
|
||||
|
||||
|
||||
def test_rgb_control_off_reads_built_setting_without_building():
|
||||
dev = _SettingsPropertyExplodes(settings_list=[_rgb_control_setting(False)])
|
||||
assert rgb_power._rgb_control_off(dev) is True
|
||||
|
||||
dev = _SettingsPropertyExplodes(settings_list=[_rgb_control_setting(True)])
|
||||
assert rgb_power._rgb_control_off(dev) is False
|
||||
|
||||
|
||||
def test_rgb_control_off_falls_back_to_persister():
|
||||
dev = _SettingsPropertyExplodes(settings_list=None, persister={"rgb_control": False})
|
||||
assert rgb_power._rgb_control_off(dev) is True
|
||||
|
||||
dev = _SettingsPropertyExplodes(settings_list=None, persister={})
|
||||
assert rgb_power._rgb_control_off(dev) is False
|
||||
|
|
|
|||
Loading…
Reference in New Issue