[Django] #19137: runserver child started via autoreload won't always exit cleanly
Django
--------------------------------------------+--------------------
Reporter: santtu | Owner: nobody
Type: Uncategorized | Status: new
Component: Core (Management commands) | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------------+--------------------
When using django.utils.autoreload (for example, runserver with
use_reloader option enabled) it is possible to get into a situation where
you might think the server has been killed, but actually is not and is
listening to the port.
Steps to reproduce:
1. Start application, when use_reloader is enabled (manage.py runserver)
2. Search for a pair of the manage.py processes (ps -aef --forest | grep
runserver etc.), make a note of which is the parent and which is the child
3. Kill the parent using command-line kill "kill -INT <parent pid>"
4. Look at ps output, and you'll find the child still alive, and via lsof
-i | grep LISTEN you can see it is still listening on the server port.
Normally if you use runserver from a terminal, pressing control-C will
send SIGINT to the whole foreground process group. Both the parent and
child will thus receive SIGINT and will exit cleanly.
If you however kill the *parent* only, the child will not receive any
signal, and thus will happily keep running.
This is a bit of a bummer when doing test automation, which tries to clean
up the test environment by killing the parent -- but the child won't go
away, and keeping server port reserved.
It is possible to do a semi-work-around by spawning runserver in its own
process group, but it requires some careful environment crafting (and I
haven't been able to get it reliably working on macos -- there are
apparently some semantic differences in setpgrp wrt/ linux and macos). And
of course is hardly obvious for anyone encountering this problem for the
first time.
I have a patch for this and I'll make a pull request and link to that when
I have a ticket id..
--
Ticket URL: <https://code.djangoproject.com/ticket/19137>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: santtu | Owner: nobody
Type: Uncategorized | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by anonymous):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
BTW, there is also another obvious work-around when doing test automation:
disable autoreloader.
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:1>
Django
Reporter: santtu | Owner: nobody
Type: Uncategorized | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by santtu):
Here's the pull request: https://github.com/django/django/pull/448
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:2>
Django
Reporter: santtu | Owner: nobody
Type: Uncategorized | Status: new
commands) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lrekucki):
* has_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:3>
Django
Reporter: santtu | Owner: nobody
Type: Bug | Status: new
Component: Core (Management | Version: 1.4
commands) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:4>
Django
Reporter: santtu | Owner: nobody
Type: Bug | Status: new
Component: Core (Management | Version: 1.4
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by ramiro):
#16982 was closed as duplicate of this one.
Feedback about how things work on Mac OS X and Windows once the PR has
been applied is welcome.
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:5>
Django
Reporter: santtu | Owner: nobody
Type: Bug | Status: new
Component: Core (Management | Version: 1.4
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by fabiofz@…):
This patch doesn't seem to work on windows.
On a quick search I found: http://danielkaes.wordpress.com/2009/06/04/how-
to-catch-kill-events-with-python/ (the last comment implies that killing a
process with the task manager in windows will kill all threads, giving no
further chance to actually treat the exception, in which case, a signal-
based approach won't work on windows).
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:6>
Django
Reporter: santtu | Owner: nobody
Type: Bug | Status: new
Component: Core (Management | Version: 1.4
commands) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
Setting patch needs improvement since it doesn't work on Windows according
to the previous comment.
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:7>
Django
Reporter: santtu | Owner: nobody
Type: Bug | Status: closed
Component: Core (Management | Version: 1.4
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* status: new => closed
* resolution: => fixed
Comment:
I think we should focus on gunicorn integration (#21978) rather than what
seems to be an obscure issue.
Actually, I think this may be fixed by #21773.
--
Ticket URL: <https://code.djangoproject.com/ticket/19137#comment:8>