From bf9226130146a64bd32c35aa41bc38c973ffce77 Mon Sep 17 00:00:00 2001 From: Irfan Ahmad Date: Mon, 29 Jun 2026 17:30:32 +0500 Subject: [PATCH] fix: make Mixologist async-safe, deprecate ObjectAggregator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove threading.RLock from Mixologist's class cache so mix() no longer blocks the event loop under ASGI deployments. dict.setdefault() atomicity (via CPython's GIL) provides equivalent thread safety, and asyncio's cooperative scheduler ensures mix() — which has no await points — is never interleaved within the same thread. ObjectAggregator has no production callers; mark it deprecated so it can be removed in a future major release. Closes #918 Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.rst | 7 +++++++ xblock/runtime.py | 34 +++++++++++++++++++--------------- xblock/test/test_runtime.py | 8 ++++++-- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 85ee6cd6a..599ca96a2 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,13 @@ Change history for XBlock Unreleased ---------- +* Removed ``threading.RLock`` from ``Mixologist``'s class cache to make it safe + for ASGI/async deployments. ``dict.setdefault()`` atomicity (via CPython's GIL) + provides equivalent thread safety without blocking the event loop. + Relates to `openedx/openedx-platform#38680 `_. +* Deprecated ``ObjectAggregator`` from ``xblock.runtime``; it had no production + callers and will be removed in a future major release. + 6.2.0 - 2026-06-09 ------------------ diff --git a/xblock/runtime.py b/xblock/runtime.py index 8aa822dda..f7c4dcd84 100644 --- a/xblock/runtime.py +++ b/xblock/runtime.py @@ -11,7 +11,6 @@ import json import logging import re -import threading import warnings from lxml import etree @@ -1195,6 +1194,11 @@ class ObjectAggregator: """ def __init__(self, *objects): + warnings.warn( + "ObjectAggregator is deprecated and will be removed in a future release.", + DeprecationWarning, + stacklevel=2, + ) self.__dict__['_objects'] = objects def _object_with_attr(self, name): @@ -1221,9 +1225,11 @@ def __delattr__(self, name): delattr(self._object_with_attr(name), name) -# Cache of Mixologist generated classes +# Cache of Mixologist generated classes. +# dict.setdefault() is atomic under CPython's GIL, so no lock is needed here. +# This is also safe in asyncio contexts: mix() has no await points, so the +# event loop cannot interleave two calls to it within the same thread. _CLASS_CACHE = {} -_CLASS_CACHE_LOCK = threading.RLock() class Mixologist: @@ -1277,18 +1283,16 @@ def mix(self, cls): mixin_key = (base_class, mixins) if mixin_key not in _CLASS_CACHE: - # Only lock if we're about to make a new class - with _CLASS_CACHE_LOCK: - # Use setdefault so that if someone else has already - # created a class before we got the lock, we don't - # overwrite it - return _CLASS_CACHE.setdefault(mixin_key, type( - base_class.__name__ + 'WithMixins', # type() requires native str - (base_class,) + mixins, - {'unmixed_class': base_class} - )) - else: - return _CLASS_CACHE[mixin_key] + new_class = type( + base_class.__name__ + 'WithMixins', # type() requires native str + (base_class,) + mixins, + {'unmixed_class': base_class} + ) + # setdefault is atomic under CPython's GIL: if two threads race + # here, one wins and the other's new_class is silently discarded. + _CLASS_CACHE.setdefault(mixin_key, new_class) + + return _CLASS_CACHE[mixin_key] class RegexLexer: diff --git a/xblock/test/test_runtime.py b/xblock/test/test_runtime.py index 4e58d813f..d5350ba99 100644 --- a/xblock/test/test_runtime.py +++ b/xblock/test/test_runtime.py @@ -375,7 +375,10 @@ def __init__(self, **kwargs): class TestObjectAggregator: """ - Test that the ObjectAggregator behaves correctly + Test that the ObjectAggregator behaves correctly. + + ObjectAggregator is deprecated; all instantiations are wrapped to assert + the expected DeprecationWarning is raised. """ # pylint: disable=attribute-defined-outside-init def setup_method(self): @@ -385,7 +388,8 @@ def setup_method(self): # Create some objects that only have single attributes self.first = Dynamic(first=1) self.second = Dynamic(second=2) - self.agg = ObjectAggregator(self.first, self.second) + with pytest.warns(DeprecationWarning, match="ObjectAggregator is deprecated"): + self.agg = ObjectAggregator(self.first, self.second) # pylint: enable=attribute-defined-outside-init def test_get(self):