emezeske/gunicorn — comment on commit 55dbf392b265f42ab893fd95f2036cb52b4f93ce
File:  line 0
By emezeske:

It's easy to reproduce various errors due to this bug. Use the following app (which I lifted directly from the Quick Start example on the gunicorn website:

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])

command:
gunicorn --bind=127.0.0.1:1234 --timeout=0 --workers=4 myapp:app

Now grab a cup of coffee. When you come back, one of many possible race conditions will have occurred, and in all likelihood gunicorn will have crashed.

You *cannot* fix this race by catching more exceptions in murder_workers(). The particular race I described in the original commit message is just the most common way for things to break; there are many other possible failure modes. Consider the fact that reap_workers() pops a worker out of WORKERS with the assumption that it's atomically removing it. That assumption is false, however, because there are several places in the main run() loop where other code may already hold a reference to the worker that is being popped out. In general, it's a very bad idea for the signal handler to modify the WORKERS hash or any of its contents.

As for the OSError being "odd", it just has to do with the timing of the signal arrival. If the signal arrives before the next call to worker.tmp.last_update(), the tmp._tmp file will be fully closed and marked as such, so that a ValueError can be generated. However, if the signal arrives *during* the call to worker.tmp.last_update(), after _tmp.fileno() has been called but before os.fstat() has been called, then an OSError(9) "bad file descriptor" is exactly what you'd expect.

Regarding the timeliness of the call to os.waitpid(), how is handling it in the main loop too slow? After handle_chld() returns, the run() loop is guaranteed to call os.waitpid() upon its next iteration. The next iteration will happen fast because handle_chld() calls wakeup().

I'm having a terribly hard time finding any reference for the fact that not calling wait() within a SIGCHLD handler can lead to a crash under FreeBSD. My understanding is that FreeBSD is fairly POSIX compliant, and it seems to me that a restriction like that would break *tons* of apps. I trust that you have a better working knowledge than I do w.r.t. FreeBSD, but I can't argue to this point without more info.