benoitc/gunicorn — pull request #624 by avaitla (2013-10-06 08:50:12+00:00)
Title: Close File Handles on HUP Signal

Thanks for Gunicorn! It's a great piece of software. I have noticed one thing which may be an issue (please let me know if I'm simply not using gunicorn correctly). Currently, when we send HUP signal to master process it creates a new logger object without closing the old one (even though reopen files is invoked), specifically every time reload is called we call setup we do "self.log=self.cfg.logger_class(app.cfg)", so we haven't closed the old descriptors, and then we call reopen on this new logger object.

Here is the guidelines to reproduce this:

Create conf.py:

bind = '127.0.0.1:2203'
accesslog = 'accesslog_test.txt'
errorlog = 'errorlog_test.txt'
pidfile = 'gunicorn.pid'
workers = 4

Create myapp.py:

def app(environ, start_response):
data = "Hello, World!\n"
start_response("200 OK", [
("Content-Type", "text/plain"),
("Content-Length", str(len(data)))
])
return iter([data])

Now start gunicorn as: gunicorn -c conf.py myapp:app.

Note the following results from lsof:

Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep accesslog_test.txt
python 11182 avaitla 5w REG 1,2 0 15340673 /Users/avaitla/gunicorn/accesslog_test.txt
Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep errorlog_test.txt
python 11182 avaitla 1u REG 1,2 3953 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 2u REG 1,2 3953 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 3u REG 1,2 3953 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 4w REG 1,2 3953 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
Anils-MacBook-Pro:gunicorn avaitla$ kill -HUP `cat gunicorn.pid`
Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep accesslog_test.txt
python 11182 avaitla 5w REG 1,2 0 15340673 /Users/avaitla/gunicorn/accesslog_test.txt
python 11182 avaitla 15w REG 1,2 0 15340673 /Users/avaitla/gunicorn/accesslog_test.txt
Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep errorlog_test.txt
python 11182 avaitla 1u REG 1,2 4576 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 2u REG 1,2 4576 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 3u REG 1,2 4576 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 4w REG 1,2 4576 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 13u REG 1,2 4576 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11182 avaitla 14w REG 1,2 4576 15340672 /Users/avaitla/gunicorn/errorlog_test.txt

Indeed, every HUP signal will further increase the number of descriptors.
Now with this pull request, we have:

Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep accesslog_test.txt
python 11255 avaitla 5w REG 1,2 0 15340673 /Users/avaitla/gunicorn/accesslog_test.txt
Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep errorlog_test.txt
python 11255 avaitla 1u REG 1,2 9964 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11255 avaitla 2u REG 1,2 9964 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11255 avaitla 3u REG 1,2 9964 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11255 avaitla 4w REG 1,2 9964 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
Anils-MacBook-Pro:gunicorn avaitla$ kill -HUP `cat gunicorn.pid`
Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep accesslog_test.txt
python 11255 avaitla 5w REG 1,2 0 15340673 /Users/avaitla/gunicorn/accesslog_test.txt
Anils-MacBook-Pro:gunicorn avaitla$ lsof -p `cat gunicorn.pid` | grep errorlog_test.txt
python 11255 avaitla 1u REG 1,2 10587 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11255 avaitla 2u REG 1,2 10587 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11255 avaitla 3u REG 1,2 10587 15340672 /Users/avaitla/gunicorn/errorlog_test.txt
python 11255 avaitla 4w REG 1,2 10587 15340672 /Users/avaitla/gunicorn/errorlog_test.txt

Note that I am reproducing a lot of the code from reopen_files and close_on_exec, so maybe we can refactor functionality so that they share this logic. I left it this way so it will be simpler to review. Thanks!