rtyler / Spawning

Spawning is a wsgi server which supports multiple processes, multiple threads, green threads, non-blocking HTTP io, and automatic graceful upgrading of code
http://pypi.python.org/pypi/Spawning
MIT License
120 stars 18 forks source link

crashing children cause pipe leakage (that's what she said) #13

Closed rdw closed 13 years ago

rdw commented 13 years ago

To repro, create the following crashtest.py:

import eventlet def app(s, e): return []

def touch_myself(): fd = open(file, 'a') fd.write("a") fd.close()

eventlet.spawn_after(1, touch_myself)

Then launch it with PYTHONPATH=. spawning --reload=dev crashtest -t 0 The reload=dev isn't a part of the bug, it's just an easy way to trigger a crash loop. When you do this, you will observe a reloading loop as the controller continually restarts its crashing children. Note that if you have a crashtest.pyc sitting around it won't work the same way, so delete that shit! Also note that you'll have to delete the errant 'a' from the end of crashtest.py before runs of this repro. I didn't say it would be easy!

While it's in the crash loop, the number of files held open by the controller will continually climb. That's incorrect.

rdw commented 13 years ago

Here's the fix:

diff --git a/src/spawning/spawning_controller.py b/src/spawning/spawning_controller.py
index 2f238c7..920d2ff 100644
--- a/src/spawning/spawning_controller.py
+++ b/src/spawning/spawning_controller.py
@@ -158,7 +158,10 @@ class Controller(object):
                     raise

             if pid and self.child_pipes.get(pid):
-                self.child_pipes.pop(pid)
+                try:  # clean up the pipe to the child 
+                    os.close(self.child_pipes.pop(pid))
+                except (IOError, OSError):
+                    pass

             if result:
                 signum = os.WTERMSIG(result)
rtyler commented 13 years ago

Fixed