Skip to content

Commit 8ce6aa9

Browse files
irfanuddinahmadIrfan Ahmadclaude
authored
fix: make Mixologist async-safe, deprecate ObjectAggregator (#934)
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: Irfan Ahmad <irfan.ahmad@A006-01919.local> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 397613a commit 8ce6aa9

3 files changed

Lines changed: 32 additions & 17 deletions

File tree

CHANGELOG.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ Change history for XBlock
55
Unreleased
66
----------
77

8+
* Removed ``threading.RLock`` from ``Mixologist``'s class cache to make it safe
9+
for ASGI/async deployments. ``dict.setdefault()`` atomicity (via CPython's GIL)
10+
provides equivalent thread safety without blocking the event loop.
11+
Relates to `openedx/openedx-platform#38680 <https://github.com/openedx/openedx-platform/issues/38680>`_.
12+
* Deprecated ``ObjectAggregator`` from ``xblock.runtime``; it had no production
13+
callers and will be removed in a future major release.
14+
815
6.2.0 - 2026-06-09
916
------------------
1017

xblock/runtime.py

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import json
1212
import logging
1313
import re
14-
import threading
1514
import warnings
1615

1716
from lxml import etree
@@ -1195,6 +1194,11 @@ class ObjectAggregator:
11951194
"""
11961195

11971196
def __init__(self, *objects):
1197+
warnings.warn(
1198+
"ObjectAggregator is deprecated and will be removed in a future release.",
1199+
DeprecationWarning,
1200+
stacklevel=2,
1201+
)
11981202
self.__dict__['_objects'] = objects
11991203

12001204
def _object_with_attr(self, name):
@@ -1221,9 +1225,11 @@ def __delattr__(self, name):
12211225
delattr(self._object_with_attr(name), name)
12221226

12231227

1224-
# Cache of Mixologist generated classes
1228+
# Cache of Mixologist generated classes.
1229+
# dict.setdefault() is atomic under CPython's GIL, so no lock is needed here.
1230+
# This is also safe in asyncio contexts: mix() has no await points, so the
1231+
# event loop cannot interleave two calls to it within the same thread.
12251232
_CLASS_CACHE = {}
1226-
_CLASS_CACHE_LOCK = threading.RLock()
12271233

12281234

12291235
class Mixologist:
@@ -1277,18 +1283,16 @@ def mix(self, cls):
12771283
mixin_key = (base_class, mixins)
12781284

12791285
if mixin_key not in _CLASS_CACHE:
1280-
# Only lock if we're about to make a new class
1281-
with _CLASS_CACHE_LOCK:
1282-
# Use setdefault so that if someone else has already
1283-
# created a class before we got the lock, we don't
1284-
# overwrite it
1285-
return _CLASS_CACHE.setdefault(mixin_key, type(
1286-
base_class.__name__ + 'WithMixins', # type() requires native str
1287-
(base_class,) + mixins,
1288-
{'unmixed_class': base_class}
1289-
))
1290-
else:
1291-
return _CLASS_CACHE[mixin_key]
1286+
new_class = type(
1287+
base_class.__name__ + 'WithMixins', # type() requires native str
1288+
(base_class,) + mixins,
1289+
{'unmixed_class': base_class}
1290+
)
1291+
# setdefault is atomic under CPython's GIL: if two threads race
1292+
# here, one wins and the other's new_class is silently discarded.
1293+
_CLASS_CACHE.setdefault(mixin_key, new_class)
1294+
1295+
return _CLASS_CACHE[mixin_key]
12921296

12931297

12941298
class RegexLexer:

xblock/test/test_runtime.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,10 @@ def __init__(self, **kwargs):
375375

376376
class TestObjectAggregator:
377377
"""
378-
Test that the ObjectAggregator behaves correctly
378+
Test that the ObjectAggregator behaves correctly.
379+
380+
ObjectAggregator is deprecated; all instantiations are wrapped to assert
381+
the expected DeprecationWarning is raised.
379382
"""
380383
# pylint: disable=attribute-defined-outside-init
381384
def setup_method(self):
@@ -385,7 +388,8 @@ def setup_method(self):
385388
# Create some objects that only have single attributes
386389
self.first = Dynamic(first=1)
387390
self.second = Dynamic(second=2)
388-
self.agg = ObjectAggregator(self.first, self.second)
391+
with pytest.warns(DeprecationWarning, match="ObjectAggregator is deprecated"):
392+
self.agg = ObjectAggregator(self.first, self.second)
389393
# pylint: enable=attribute-defined-outside-init
390394

391395
def test_get(self):

0 commit comments

Comments
 (0)