-
Notifications
You must be signed in to change notification settings - Fork 76
Description
I came across a marshmallow-dataclass-related race condition in my new code that was resulting in the marshmallow exception when using class_schema()
while marshalling and unmarshalling dataclasses from threads of a multi-threaded application:
marshmallow.exceptions.RegistryError: Class with name '{{a marshmallow data class that I defined}}' was not found. You may need to import the class
I root caused the exception to the following. My code was invoking marshmallow_dataclass.class_schema() from threads:
request = class_schema({{any one of a number of my dataclasses}})().load(…)
It turns out that class_schema() may invoke marshmallow.class_registry.register(), which is not thread-safe. It performs operations on a global dict
, and marshmallow.class_registry.register()
is in excess of a single GIL-protected byte code.
I have not yet come up with a simple test case, but it's easy to see via simple code inspection that marshmallow.class_registry.register()
(and class_schema() as a result) is not thread-safe: https://github.com/marshmallow-code/marshmallow/blob/a651fbcb85f4a2fc1bbc9df168e66476854fe178/src/marshmallow/class_registry.py#L26-L69.
I was able to work around the race condition by forcing generation of the schema at import time by calling class_schema()
on all of my dataclasses at the top level of the module. However, in a large code base with multiple contributors, it's nearly impossible to ensure that a workaround like that won't be missed and show up in production, since some of the scenarios might not trigger the race condition in the test environment, but would show up in production later.
On a related note:
- Why is generation of the Schema deferred until
.Schema()
orclass_schema()()
is invoked? Presumably, if someone is usingmarshmallow_dataclass.dataclass
, they will be marshalling and/or unmarshalling with that class, so pre-generating the schema wouldn't be a waste. And if they are not using it for marshalling/unmarshalling, then they should just be using the builtindataclasses.dataclass
, instead. - If there is not going to be a fix for this race condition, what's a full proof workaround (that's not error-prone)?