- Spurious errors related to unclosed files ("bad file descriptor",

    typically) were evident at supervisord "reload" time (when using
    the "reload" command from supervisorctl).

CHANGES.txt

@@ -235,6 +235,10 @@ Next Release
     http://www.repoze.org/LICENSE.txt; it's slightly less restrictive
     than the ZPL (no servicemark clause).
 
+  - Spurious errors related to unclosed files ("bad file descriptor",
+    typically) were evident at supervisord "reload" time (when using
+    the "reload" command from supervisorctl).
+
 3.0a2
 
   - Fixed the README.txt example for defining the supervisor RPC

src/supervisor/loggers.py

@@ -70,11 +70,15 @@ class Handler:
         try:
             self.stream.flush()
         except IOError, why:
-            # if supervisor output is piped, this can be raised at exit
+            # if supervisor output is piped, EPIPE can be raised at exit
             if why[0] != errno.EPIPE:
                 raise
 
     def close(self):
+        if hasattr(self.stream, 'fileno'):
+            fd = self.stream.fileno()
+            if fd < 3: # don't ever close stdout or stderr
+                return
         self.stream.close()
 
     def emit(self, record):
@@ -194,7 +198,14 @@ class RotatingFileHandler(FileHandler):
         """
         if self.maxBytes <= 0:
             return
-        
+
+        try:
+            pos = self.stream.tell()
+        except IOError, why:
+            # Attempt to trap IOError: [Errno 29] Illegal seek
+            print self.baseFilename, self.maxBytes, self.stream
+            raise
+            
         if not (self.stream.tell() >= self.maxBytes):
             return
 
@@ -245,6 +256,10 @@ class Logger:
             handlers = []
         self.handlers = handlers
 
+    def close(self):
+        for handler in self.handlers:
+            handler.close()
+
     def blather(self, msg, **kw):
         if LevelsByName.BLAT >= self.level:
             self.log(LevelsByName.BLAT, msg, **kw)

src/supervisor/options.py

@@ -885,6 +885,9 @@ class ServerOptions(Options):
         for config, server in self.httpservers:
             server.close()
 
+    def close_logger(self):
+        self.logger.close()
+
     def setsignals(self):
         signal.signal(signal.SIGTERM, self.sigreceiver)
         signal.signal(signal.SIGINT, self.sigreceiver)
@@ -940,14 +943,8 @@ class ServerOptions(Options):
         return asyncore.socket_map
 
     def cleanup_fds(self):
-        # try to close any unused file descriptors to prevent leakage.
-        # we start at the "highest" descriptor in the asyncore socket map
-        # because this might be called remotely and we don't want to close
-        # the internet channel during this call.
-        asyncore_fds = asyncore.socket_map.keys()
+        # try to close any leaked file descriptors (for reload)
         start = 5
-        if asyncore_fds:
-            start = max(asyncore_fds) + 1
         for x in range(start, self.minfds):
             try:
                 os.close(x)

src/supervisor/supervisord.py

@@ -317,6 +317,7 @@ def main(args=None, test=False):
         if test or (options.mood < SupervisorStates.RESTARTING):
             break
         options.close_httpservers()
+        options.close_logger()
         first = False
 
 def go(options):

src/supervisor/tests/base.py

@@ -267,7 +267,6 @@ class DummyLogger:
     def getvalue(self):
         return ''.join(self.data)
 
-
 class DummySupervisor:
     def __init__(self, options=None, state=None, process_groups=None):
         if options is None:

src/supervisor/tests/test_loggers.py

@@ -271,12 +271,22 @@ class LoggerTests(unittest.TestCase):
         logger.critical('hello')
         self.assertEqual(len(handler.records), 1)
 
+    def test_close(self):
+        from supervisor.loggers import LevelsByName
+        handler = DummyHandler(LevelsByName.CRIT)
+        logger = self._makeOne(LevelsByName.CRIT, (handler,))
+        logger.close()
+        self.assertEqual(handler.closed, True)
+
 class DummyHandler:
+    close = False
     def __init__(self, level):
         self.level = level
         self.records = []
     def emit(self, record):
         self.records.append(record)
+    def close(self):
+        self.closed = True
 
 def test_suite():
     return unittest.findTestCases(sys.modules[__name__])

src/supervisor/tests/test_options.py

@@ -305,6 +305,24 @@ class ServerOptionsTests(unittest.TestCase):
             except OSError:
                 pass
 
+    def test_close_httpservers(self):
+        instance = self._makeOne()
+        class Server:
+            closed = False
+            def close(self):
+                self.closed = True
+        server = Server()
+        instance.httpservers = [({}, server)]
+        instance.close_httpservers()
+        self.assertEqual(server.closed, True)
+        
+    def test_close_logger(self):
+        instance = self._makeOne()
+        logger = DummyLogger()
+        instance.logger = logger
+        instance.close_logger()
+        self.assertEqual(logger.closed, True)
+
     def test_write_pidfile_ok(self):
         fn = tempfile.mktemp()
         try: