jezdez / envdir

A Python port of daemontools' envdir.
https://envdir.readthedocs.io/
MIT License
229 stars 28 forks source link

envdir doesn't propagate TERM event properly to subprocess. #14

Closed zerok closed 10 years ago

zerok commented 10 years ago

We yesterday discovered an issue where for some reason SIGTERM is not cleanly propagated to the child process.

For instance, if you launch a gunicorn instance with two workers

$ envdir env python manage.py run_gunicorn -w 2
2013-12-20 08:51:00 [9004] [INFO] Starting gunicorn 18.0
2013-12-20 08:51:00 [9004] [INFO] Listening at: http://127.0.0.1:8000 (9004)
2013-12-20 08:51:00 [9004] [INFO] Using worker: sync
2013-12-20 08:51:00 [9007] [INFO] Booting worker with pid: 9007
2013-12-20 08:51:00 [9008] [INFO] Booting worker with pid: 9008

$ % ps aux | grep envdir
zerok            9104   0,0  0,1  2449096   8436 s000  S+    8:52am   0:00.08 /usr/local/Cellar/python/2.7.5/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /usr/local/bin/envdir env python manage.py run_gunicorn -w 2

And then send SIGTERM to the envdir process, you end up with only this process terminating:

$ kill -TERM 9104
$  ps aux | grep gunicorn
zerok            9165   0,0  0,0  2442000    576 s002  R+    8:54am   0:00.00 grep gunicorn
zerok            9109   0,0  0,5  2508164  44144 s000  S     8:52am   0:00.49 python manage.py run_gunicorn -w 2
zerok            9108   0,0  0,5  2498948  43704 s000  S     8:52am   0:00.51 python manage.py run_gunicorn -w 2
zerok            9105   0,0  0,3  2483984  27540 s000  S     8:52am   0:00.34 python manage.py run_gunicorn -w 2

I couldn't reproduce this directly with deamontools' envdir because they seem to completely replace the envdir process once the environment is set up.

jezdez commented 10 years ago

Can you first try this by using the gunicorn tool instead of going through the Django management command run_gunicorn (which is deprecated)?

zerok commented 10 years ago

Same behaviour as far as I can tell. What is the main motivation behind using subprocess.Popen instead of os.execvpe?

jezdez commented 10 years ago

@zerok Okay, thanks for trying. No particular reason, does "os.execvpe" work on Windows?

Also, can you try #15 please?

jezdez commented 10 years ago

This is now fixed and released as 0.6.1. Thanks @zerok!