daemonize should close all open files

[Paul Egan]

In source:django/utils/daemonize.py, 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.

[Paul Egan]

Patch for become_daemon to close all open files

[Malcolm Tredinnick]

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.

[Jacob]

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

[Malcolm Tredinnick]

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.

[Paul Egan]

The [source:django/trunk/django/utils/daemonize.py become_daemon] function has two definitions, switched on os.name. The change only applies to the os.name == '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 os.name == 'nt', and cygwin python (and other posix-based ports) do have os.sysconf defined.

[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.

[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.

[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?

[(none)]

Milestone post-1.0 deleted

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Tim Graham]

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