Opened 11 years ago

Closed 6 years ago

#8540 closed Bug (wontfix)

daemonize should close all open files

Reported by: Paul Egan Owned by: nobody
Component: Core (Management commands) Version: master
Severity: Normal Keywords: daemonize become_daemon fastcgi
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


In source:django/utils/, the become_daemon function should close all open file descriptors, not just the standard three. Without this, a fastcgi process can hold open files or sockets from the parent process.

Attachments (1)

daemonize.diff (617 bytes) - added by Paul Egan 11 years ago.
Patch for become_daemon to close all open files

Download all attachments as: .zip

Change History (15)

Changed 11 years ago by Paul Egan

Attachment: daemonize.diff added

Patch for become_daemon to close all open files

comment:1 Changed 11 years ago by Malcolm Tredinnick

milestone: 1.0
Triage Stage: UnreviewedAccepted

Good practice, although there's no sensible way of running that program which will have extra open file-descriptors, which is why it isn't doing that already. Won't hurt to protect people from themselves, though.

comment:2 Changed 11 years ago by Malcolm Tredinnick

Triage Stage: AcceptedReady for checkin

comment:3 Changed 11 years ago by Jacob

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

os.sysconf isn't available on windows, so this needs to work around that somehow.

comment:4 Changed 11 years ago by Malcolm Tredinnick

milestone: 1.0post-1.0

Pushing to after 1.0. Don't know what I was thinking this morning when I set the milestone here. If we get this wrong, it destabilises things.

comment:5 Changed 11 years ago by Paul Egan

The [source:django/trunk/django/utils/ become_daemon] function has two definitions, switched on The change only applies to the == 'posix' version, for which os.sysconf should always be defined.

I don't normally use Windows, but I quickly checked: the native python install on Windows has == 'nt', and cygwin python (and other posix-based ports) do have os.sysconf defined.

comment:6 Changed 11 years ago by Dan Watson

I also ran into this issue when spawning a daemon from a view to build some reports. Basically, the view called a script using os.system, and the script set DJANGO_SETTINGS_MODULE and did some processing using django, after calling become_daemon. The os.system call would return immediately, but it looked like django would not finish it's request processing until after the spawned script finished. Applying this patch fixed the problem.

I don't fully understand the black art of daemonization, but it seems like this may not even be an issue on Windows. Nothing is being forked, there is no "parent process", it's just redirecting stdin, stdout, and stderr. Regardless, it seems like fixing a bug for posix systems, even if still exists on Windows, is better for 1.0 than leaving the bug for both.

comment:7 Changed 11 years ago by Malcolm Tredinnick

@dcwatson: yours is a case of using our internal daemonisation process for external usage. That's definitely a situation where closing the external descriptors would be necessary. We've written that thing for internal usage only, but once again people have fallen for the "everything in django.utils.* is intended for public reuse trap (it's not).

This is a case where Django really shouldn't be supplying that stuff for public reuse; it's not our domain. We're providing a web/database framework, not something that provides extra utilities like daemonising functionality -- those are better provided by other frameworks. This patch would help bring django.utils.daemonise up to that level of utility, but for 1.0 that isn't a goal. The patch in some form will go in post-1.0, but it's not a necessary bug fix at the moment (and the API stability document for 1.0 will make it clear what portions of django.utils are available for public usage.

I realise it looks like a simple case and what could possibly go wrong? But it's a feature addition: there is no bug in the way this thing is used in Django and it is not intended as public API.

comment:8 Changed 11 years ago by Dan Watson

Fair enough. I agree, if it's not meant for public consumption, no need to fix it for yahoos like me using it publicly :-) Looking forward to the API stability document. I admit I was thrown by the docstring on the function -- "Robustly turn into a UNIX daemon..." Is it worth documenting these types of things as non-public in code as well as the API stability doc?

comment:9 Changed 10 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:10 Changed 8 years ago by Luke Plant

Severity: Normal
Type: Bug

comment:11 Changed 8 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:12 Changed 8 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:13 Changed 6 years ago by Aymeric Augustin

Component: Core (Other)Core (Management commands)

comment:14 Changed 6 years ago by Tim Graham

Resolution: wontfix
Status: newclosed

runfcgi is deprecated in Django 1.7 and that appears to be the only code in Django that uses django/utils/

Note: See TracTickets for help on using tickets.
Back to Top