Hi Peter, On 11/12/2014 10:59 PM, Peter Sagerson wrote: > I agree that setting self.exclude will set the instance attribute, > thus hiding the class attribute of the same name. However, it appears > that a given AdminSite instantiates each registered ModelAdmin once > at registration time.[1] The default AdminSite itself is a module > global. Thus, any long-running, multi-threaded Django application > server (e.g. a threaded uWSGI process) might be relying on a given > ModelAdmin instance to serve multiple requests concurrently. Neither > AdminSite nor ModelAdmin appear to have any thread-locality, > suggesting that modifying self.exclude (or anything else) for an > individual request is a race condition waiting to happen. > > I believe this is essentially a subtle documentation bug, although > it's worth asking whether there are any deeper assumptions about > ModelAdmin or AdminSite having any kind of request isolation.
Your analysis is entirely correct, as far as I can tell. Setting attributes on `self` in a per-request method of a `ModelAdmin` is not concurrency-safe, and it should not be recommended or demonstrated in the docs. If you'd be willing to file a bug (or, better, a PR) to fix this documentation example, that would be excellent. I don't know of any bugs of this type in Django itself, but it's certainly possible there are some. It shouldn't be too hard to audit for them - if you find one, definitely file it as an issue. Unfortunately I would expect bugs like this to become only more common in user code going forward, since the class-based-views framework does behind-the-scenes magic to make `self` concurrency-safe in CBVs, so it's likely that users will transfer those expectations to their `ModelAdmin` classes. Thanks! Carl
signature.asc
Description: OpenPGP digital signature
