Opened 9 years ago

Closed 9 years ago

#6175 closed (fixed)

become_daemon should use os._exit instead of sys.exit

Reported by: tclugg@… Owned by: Leo Shklovskii
Component: Core (Other) Version: master
Severity: Keywords: daemon, daemonize, become_daemon, segfault
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

django.utils.daemonize.become_daemon forks then uses sys.exit(0) instead of os._exit(0), which can cause some nasty issues. os._exit is the correct way to exit after forking in Python, as os._exit avoids calling Python cleanup functions which may cause issues such as segfaults.

See http://docs.python.org/lib/os-process.html for details on os._exit().

Attachments (2)

daemon.patch (793 bytes) - added by Tyson Clugg <tclugg@…> 9 years ago.
6175_second_patch.diff (477 bytes) - added by Leo Shklovskii 9 years ago.

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by Tyson Clugg <tclugg@…>

Attachment: daemon.patch added

comment:1 Changed 9 years ago by anonymous

Summary: become_daemon should use os._exit instead of sys.exit[patch] become_daemon should use os._exit instead of sys.exit

comment:2 Changed 9 years ago by Chris Beaven

Summary: [patch] become_daemon should use os._exit instead of sys.exitbecome_daemon should use os._exit instead of sys.exit
Triage Stage: UnreviewedDesign decision needed

Pushing to a design decision as it's over my head :)

comment:3 Changed 9 years ago by Malcolm Tredinnick

Triage Stage: Design decision neededAccepted

This is only half right. The main parent should call sys.exit(), since it can be cleaned up normally; it's a normal short-lived process. The children should generally call os._exit() to avoid double cleanups via atexit() paths.

So I believe the first change is wrong and the second change is right, but I need to do some re-reading to confirm this.

Changed 9 years ago by Leo Shklovskii

Attachment: 6175_second_patch.diff added

comment:4 Changed 9 years ago by Leo Shklovskii

Owner: changed from nobody to Leo Shklovskii
Status: newassigned

Malcolm, you're right. The first exit() call is in the parent so it should stay exit(). The second call is in the first child, so _exit() is more appropriate. I've attached a patch only to correct the second one.

comment:5 Changed 9 years ago by Malcolm Tredinnick

Resolution: fixed
Status: assignedclosed

(In [7290]) Fixed #6175 -- Use os._exit() instead of sys.exit() in child processes. Based
on a patch from Tyson Clugg.

comment:6 Changed 9 years ago by durdinator

Resolution: fixed
Status: closedreopened

Malcolm wrote:

So I believe the first change is wrong and the second change is right, but I need to do some re-reading to confirm this.

and Leo wrote:

Malcolm, you're right. The first exit() call is in the parent so it should stay exit().

But the patch actually committed changes both calls. Are you sure that's correct?

comment:7 Changed 9 years ago by Malcolm Tredinnick

No it didn't. Have a look at the line numbers affected. The patch I committed was a variant on the two attached to these tickets and fixed a place that was overlooked in both those patches.

comment:8 Changed 9 years ago by durdinator

Resolution: fixed
Status: reopenedclosed

Ah, I see. Glad to see it wasn't a mistake after all.

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