[Django] #26152: django.setup() hangs django if ever used in a module that's imported by an app
Django
app
-------------------------------+--------------------
Reporter: hjwp | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The docs suggest using django.setup() for any script that needs to be run
standalone:
https://docs.djangoproject.com/en/1.9/topics/settings/#calling-django-
setup-is-required-for-standalone-django-usage
But this causes problems if a standalone script also contains some code
that is imported by a regular django app.
Here's a minimal repro:
if foo.py contains django.setup(), and myapp.models.py tries to import
from foo.py, that now causes a hang such that no manage.py call will ever
complete. ouch!
The solution/workaround is to put any calls to django.setup() inside a
{{{
if __name__ == '__main__':
import django
django.setup()
}}}
I gather that fixing django.setup() / apps.populate() to make it reentrant
is hard. So this could be a quick documentation fix instead?
cf #18251, a more complex version of this bug I think, which is in fact
quite simple to run into...
--
Ticket URL: <https://code.djangoproject.com/ticket/26152>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
app
Reporter: hjwp | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.9
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hjwp):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
I want to submit a documentation fix, but I'm not sure where. The new
docs don't really have a section that specifically recommends using
django.setup() for standalone scripts?
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:1>
Django
app
Reporter: hjwp | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.9
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
aha, there it is, in docs/ref/applications.txt.
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:2>
Django
app
Reporter: hjwp | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* type: Uncategorized => Cleanup/optimization
* component: Uncategorized => Documentation
Comment:
How about in the topics/settings section that you linked to in the
description? Something like, "When writing standlone scripts, keep them as
minimal as possible and put application logic in other modules of your
application. You can't import scripts that call `django.setup()` in your
application as this will lead to a deadlock."
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:3>
Django
app
Reporter: hjwp | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
hi tim, yes, sorry, i was looking at an out-of-date code tree...
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:4>
Django
app
Reporter: hjwp | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
how about this? https://github.com/django/django/pull/6060
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:5>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
I don't think it's a good practice to write "standalone scripts" that are
imported Django itself.
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:6>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
We may be able to detect that case and raise an exception instead of
hanging. (I didn't look at the code too closely.)
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:7>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
Hi Aymeric, there's a couple of ways I can think of that would detect
cycles and then a couple of things we could do with it:
detect options:
- import the traceback module and examine the call stack for a previous
call to django.setup()
- set some global variable in the module, _setup_called = True, or
whatever it may be. (there is already a class variable called .ready,
which is set to true at the end of the apps.populate() call. So maybe we
could just add a second class variable that's set at the beginning)
what to do:
- raise an exception
- or, just an early return. Maybe setup() should just be idempotent
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:8>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
Just been digging into this. And I am not an experienced coder of
multithreaded code, so I probably shouldn't be allowed.
Here's the current code:
{{{
# populate() might be called by two threads in parallel on servers
# that create threads before initializing the WSGI callable.
with self._lock:
if self.ready:
return
# app_config should be pristine, otherwise the code below
won't
# guarantee that the order matches the order in
INSTALLED_APPS.
if self.app_configs:
raise RuntimeError("populate() isn't reentrant")
# [...] rest of populate code
self.ready = True
}}}
So switching _lock to being a threading.RLock will have the desired effect
of raising the "populate isn't reentrant" exception that's already been
defined, if populate is ever called recursively by the same thread.
I'm not sure I understand the code though. Under what circumstances, with
the current threading.Lock(), would we expect the RuntimeError to be
encountered?
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:9>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Yes, I was thinking of adding a boolean flag when the process starts.
I /think/ we discussed using a RLock before and rejected it, but I'm not
sure. I have to investigate. I'll try to find time.
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:10>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
I might be wrong but, given the use of threading.Lock(), isn't the
RunTimeError if self.app_configs theoretically impossible?
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:11>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"0fb1185538aeec9004fb9c84d96b81dc2778f66a" 0fb1185]:
{{{
#!CommitTicketReference repository=""
revision="0fb1185538aeec9004fb9c84d96b81dc2778f66a"
Fixed #26152 -- Documented how to avoid django.setup() deadlock in
standalone scripts.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:12>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
Hi all, can I propose a re-open? We just got bitten by this thing today
again, where calling django.setup() was messing with the logging setup in
weird and unpredictable ways in our unit tests.
It seems like the ideal version of django.setup() should be idempotent, or
only able to be called once?
Or should I open a different ticket?
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:13>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"fe41a134bcecd0fc7b6905a46bf055fca100fe28" fe41a134]:
{{{
#!CommitTicketReference repository=""
revision="fe41a134bcecd0fc7b6905a46bf055fca100fe28"
[1.9.x] Fixed #26152 -- Documented how to avoid django.setup() deadlock in
standalone scripts.
Backport of 0fb1185538aeec9004fb9c84d96b81dc2778f66a from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:14>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I guess a new ticket is appropriate since the "Documentation" bit has been
completed.
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:15>
Django
app
Reporter: hjwp | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: Documentation | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Harry: eventually I found the time to investigate this issue and filed
#27176. Feel free to comment on that issue if you have anything to add.
--
Ticket URL: <https://code.djangoproject.com/ticket/26152#comment:16>