From 5c357184680ad5b0d32af5818d381d4472fadccc Mon Sep 17 00:00:00 2001 From: genisis0x Date: Tue, 12 May 2026 15:38:31 +0530 Subject: [PATCH] fix(config): keep QlibConfig.registered safe in joblib worker subprocesses QlibConfig.registered is implemented as a property that reads self._registered. Because the overridden __setattr__ routes attribute writes through self.__dict__["_config"], "_registered" lives inside self._config rather than on the instance __dict__. If self._config has been rebuilt from _default_config (which does not contain "_registered") -- for example in a freshly spawned joblib worker that re-imports qlib, or after Config.reset() inside QlibConfig.__init__ before QlibConfig sets self._registered -- the property's inner self._registered access raises AttributeError, Python falls back to Config.__getattr__ for the original name ("registered"), and the worker crashes with: AttributeError: No such ``registered`` in self._config This terminates backtests that spawn joblib workers along the PortfolioMetrics._cal_benchmark -> get_higher_eq_freq_feature -> D.features -> ParallelExt -> inst_calculator -> register_from_C path (issue #2038). Make the property defensive: read "_registered" out of self.__dict__["_config"] with a safe ``get`` chain so a missing key returns False instead of triggering the property -> __getattr__ cascade. Also harden register_from_C with ``getattr(C, "registered", False)`` so the second top-level caller is double-protected. Add regression tests for: - registered returns False when "_registered" key is absent from _config, - registered returns False on a partially-initialised QlibConfig (mirrors unpickle / fresh-import state seen in workers), - ``getattr(C, "registered", False)`` does not raise when the key is missing on the global C, - registered returns the assigned value on the happy path so the fix does not regress normal behaviour. Fixes #2038 --- qlib/config.py | 20 +++++++- tests/misc/test_config_registered.py | 70 ++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 tests/misc/test_config_registered.py diff --git a/qlib/config.py b/qlib/config.py index ae05037e2ff..7f5841af2de 100644 --- a/qlib/config.py +++ b/qlib/config.py @@ -112,7 +112,16 @@ def set_conf_from_C(self, config_c): def register_from_C(config, skip_register=True): from .utils import set_log_with_config # pylint: disable=C0415 - if C.registered and skip_register: + # ``C.registered`` resolves through ``QlibConfig.registered``, which + # reads ``self._registered`` -- a key stored inside ``self._config`` via + # the overridden ``__setattr__``. In freshly-spawned joblib worker + # processes that key may be absent (e.g. when ``Config.reset()`` rebuilt + # ``_config`` from ``_default_config`` between ``QlibConfig.__init__`` + # and this call), and ``Config.__getattr__`` would then raise + # ``AttributeError: No such ``registered`` in self._config`` and crash + # the backtest worker (issue #2038). Falling back to ``False`` lets + # the worker re-register safely instead of dying. + if getattr(C, "registered", False) and skip_register: return C.set_conf_from_C(config) @@ -520,7 +529,14 @@ def get_kernels(self, freq: str): @property def registered(self): - return self._registered + # ``_registered`` is stored inside ``self._config`` because the + # overridden ``__setattr__`` routes attribute writes there. After a + # ``Config.reset()`` (or in joblib worker subprocesses where ``_config`` + # is recreated from ``_default_config`` before ``QlibConfig.__init__`` + # finishes), the key may be missing. Treat missing as ``False`` so + # callers see a consistent "not yet registered" signal instead of + # ``AttributeError`` (issue #2038). + return self.__dict__.get("_config", {}).get("_registered", False) # global config diff --git a/tests/misc/test_config_registered.py b/tests/misc/test_config_registered.py new file mode 100644 index 00000000000..d3e6fb79bb0 --- /dev/null +++ b/tests/misc/test_config_registered.py @@ -0,0 +1,70 @@ +"""Regression tests for issue #2038. + +In freshly spawned joblib worker processes ``register_from_C`` reads +``C.registered`` before ``set_conf_from_C`` syncs the parent's config. When +``_config`` does not carry the ``_registered`` key (for instance after a +``Config.reset()`` call), the previous implementation surfaced as +``AttributeError: No such ``registered`` in self._config`` and crashed the +backtest worker. The fixed property and the defensive ``getattr`` in +``register_from_C`` must keep these scenarios safe. +""" + +from __future__ import annotations + +import unittest + +from qlib.config import C, Config, QlibConfig, _default_config + + +class QlibConfigRegisteredAccessTest(unittest.TestCase): + """Issue #2038: ``C.registered`` must never raise even if the + ``_registered`` key was scrubbed from ``self._config``.""" + + def test_registered_property_returns_false_when_key_missing(self) -> None: + config = QlibConfig(_default_config) + # Simulate the worker state where ``_config`` was rebuilt from + # ``_default_config`` (which does not contain ``_registered``). + config.__dict__["_config"].pop("_registered", None) + + try: + value = config.registered + except AttributeError as exc: # pragma: no cover - failure path + self.fail(f"QlibConfig.registered raised AttributeError: {exc}") + + self.assertFalse(value) + + def test_registered_property_returns_false_when_config_missing(self) -> None: + # Defensive: even if ``_config`` itself has not been initialised yet + # (e.g. partially-constructed instance during unpickling), the property + # must surface ``False`` rather than blow up. + config = QlibConfig.__new__(QlibConfig) + self.assertFalse(config.registered) + + def test_register_from_C_does_not_raise_when_registered_missing(self) -> None: + original_config = dict(C.__dict__["_config"]) + original_registered = C.__dict__["_config"].pop("_registered", False) + try: + # Should hit the defensive ``getattr`` path and NOT raise. We do + # not call ``register_from_C`` end-to-end (it would touch the + # workflow exit handler) -- instead we exercise the guarded + # boolean expression directly, which is the exact line that + # raised in the original bug report. + self.assertFalse(getattr(C, "registered", False)) + finally: + # Restore C so other tests are not affected. + C.__dict__["_config"].clear() + C.__dict__["_config"].update(original_config) + if original_registered is not None: + C.__dict__["_config"]["_registered"] = original_registered + + def test_registered_property_after_normal_construction(self) -> None: + # The defensive change must not regress the happy path. + config = QlibConfig(_default_config) + self.assertFalse(config.registered) + + config.__dict__["_config"]["_registered"] = True + self.assertTrue(config.registered) + + +if __name__ == "__main__": + unittest.main()