Commits

Robert Brewer committed f03a210

Fix for #751 (logging: Python file objects are not thread-safe). Whew. Finally got all the bus operations where I want em:

1. There's a new EXITING state
2. bus.block() waits for EXITING now instead of STOPPED, and also wait for all threads to exit, so the main thread doesn't exit (and run atexit callbacks) too early.
3. bus.exit() no longer calls sys.exit. Instead, exit is used to signal the main thread to unblock and terminate normally. This means some callers of stop need(ed) to be changed to call exit instead.
4. bus.block() no longer takes a state arg; it's now only for use by the main thread. Call bus.wait(state) to wait for other states.

Comments (0)

Files changed (8)

cherrypy/_cpmodpy.py

     cherrypy.engine.start()
     
     def cherrypy_cleanup(data):
-        cherrypy.engine.stop()
+        cherrypy.engine.exit()
     try:
         from mod_python import apache
         # apache.register_cleanup wasn't available until 3.1.4.

cherrypy/restsrv/servers.py

         except KeyboardInterrupt, exc:
             self.bus.log("<Ctrl-C> hit: shutting down HTTP server")
             self.interrupt = exc
-            self.bus.stop()
+            self.bus.exit()
         except SystemExit, exc:
             self.bus.log("SystemExit raised: shutting down HTTP server")
             self.interrupt = exc
-            self.bus.stop()
+            self.bus.exit()
             raise
         except:
             import sys
             self.interrupt = sys.exc_info()[1]
             self.bus.log("Error in HTTP server: shutting down",
                          traceback=True)
-            self.bus.stop()
+            self.bus.exit()
             raise
     
     def wait(self):

cherrypy/restsrv/win32.py

             except ValueError:
                 pass
             
-            self.stop()
+            self.exit()
             # 'First to return True stops the calls'
             return 1
         return 0
         win32event.PulseEvent(event)
     state = property(_get_state, _set_state)
     
-    def block(self, state=wspbus.states.STOPPED, interval=1):
+    def wait(self, state, interval=0.1):
         """Wait for the given state, KeyboardInterrupt or SystemExit.
         
         Since this class uses native win32event objects, the interval
         argument is ignored.
         """
         # Don't wait for an event that beat us to the punch ;)
-        if self.state == state:
-            return
-        
-        event = self._get_state_event(state)
-        try:
+        if self.state != state:
+            event = self._get_state_event(state)
             win32event.WaitForSingleObject(event, win32event.INFINITE)
-            if self.execv:
-                self._do_execv()
-        except SystemExit:
-            self.log('SystemExit raised: shutting down bus')
-            self.stop()
-            raise
 
 
 class _ControlCodes(dict):
     def SvcStop(self):
         from cherrypy import restsrv
         self.ReportServiceStatus(win32service.SERVICE_STOP_PENDING)
-        restsrv.bus.stop()
+        restsrv.bus.exit()
     
     def SvcOther(self, control):
         restsrv.bus.publish(control_codes.key_for(control))

cherrypy/restsrv/wspbus.py

                         O
                         |
                         V
-       STOPPING --> STOPPED --> X
+       STOPPING --> STOPPED --> EXITING -> X
           A   A         |
           |    \___     |
           |        \    |
 states.STARTING = states.State()
 states.STARTED = states.State()
 states.STOPPING = states.State()
+states.EXITING = states.State()
 
 
 class Bus(object):
             start_trace =  _traceback.format_exc()
             e_info = sys.exc_info()
             try:
-                self.stop()
+                self.exit()
             except:
-                # Any stop errors will be logged inside publish().
+                # Any stop/exit errors will be logged inside publish().
                 pass
             self.log("Exception that caused shutdown: %s" % start_trace)
             raise e_info[0], e_info[1], e_info[2]
     
-    def exit(self, status=0):
-        """Stop all services and exit the process."""
+    def exit(self):
+        """Stop all services and prepare to exit the process."""
         self.stop()
+        
+        self.state = states.EXITING
         self.log('Bus exit')
         self.publish('exit')
-        sys.exit(status)
     
     def restart(self):
         """Restart the process (may close connections).
         instead, it stops the bus and asks the main thread to call execv.
         """
         self.execv = True
-        self.stop()
+        self.exit()
     
     def graceful(self):
         """Advise all services to reload."""
         self.log('Bus graceful')
         self.publish('graceful')
     
-    def block(self, state=states.STOPPED, interval=0.1):
-        """Wait for the given state, KeyboardInterrupt or SystemExit."""
+    def block(self, interval=0.1):
+        """Wait for the EXITING state, KeyboardInterrupt or SystemExit."""
         try:
-            while self.state != state:
-                time.sleep(interval)
-            if self.execv:
-                self._do_execv()
+            self.wait(states.EXITING, interval=interval)
         except (KeyboardInterrupt, IOError):
             # The time.sleep call might raise
             # "IOError: [Errno 4] Interrupted function call" on KBInt.
             self.log('Keyboard Interrupt: shutting down bus')
-            self.stop()
+            self.exit()
         except SystemExit:
             self.log('SystemExit raised: shutting down bus')
-            self.stop()
+            self.exit()
             raise
+        
+        # Waiting for ALL child threads to finish is necessary on OS X.
+        # See http://www.cherrypy.org/ticket/581.
+        # It's also good to let them all shut down before allowing
+        # the main thread to call atexit handlers.
+        # See http://www.cherrypy.org/ticket/751.
+        self.log("Waiting for child threads to terminate...")
+        for t in threading.enumerate():
+            if (t != threading.currentThread() and t.isAlive()
+                # Note that any dummy (external) threads are always daemonic.
+                and not t.isDaemon()):
+                t.join()
+        
+        if self.execv:
+            self._do_execv()
+    
+    def wait(self, state, interval=0.1):
+        """Wait for the given state."""
+        while self.state != state:
+            time.sleep(interval)
     
     def _do_execv(self):
-        """Re-execute the current process."""
-        self.execv = False
+        """Re-execute the current process.
         
-        self.log('Bus restart')
-        self.publish('exit')
-        
-        # Re-execute the current process. We must do this in the
-        # main thread (which is the only thread that should be
-        # calling block) because OS X doesn't allow execv to be
-        # called in a child thread very well.
-        # Waiting for ALL child threads to finish is necessary on OS X.
-        # See: http://www.cherrypy.org/ticket/581
-        self.log("Waiting for child threads to terminate...")
-        for t in threading.enumerate():
-            if t != threading.currentThread():
-                t.join()
-        
+        This must be called from the main thread, because certain platforms
+        (OS X) don't allow execv to be called in a child thread very well.
+        """
         args = sys.argv[:]
         self.log('Re-spawning %s' % ' '.join(args))
         args.insert(0, sys.executable)
-        
         if sys.platform == 'win32':
             args = ['"%s"' % arg for arg in args]
         
         args = (func,) + args
         
         def _callback(func, *a, **kw):
-            self.block(states.STARTED)
+            self.wait(states.STARTED)
             func(*a, **kw)
         t = threading.Thread(target=_callback, args=args, kwargs=kwargs)
         t.setName('Bus Callback ' + t.getName())

cherrypy/test/benchmark.py

             try:
                 run_standard_benchmarks()
             finally:
-                cherrypy.engine.stop()
+                cherrypy.engine.exit()
                 cherrypy.server.stop()
     
     print "Starting CherryPy app server..."

cherrypy/test/helper.py

     cherrypy.server.httpserver.wsgi_app.apps = apps
 
 def _run_test_suite_thread(moduleNames, conf):
-    for testmod in moduleNames:
-        # Must run each module in a separate suite,
-        # because each module uses/overwrites cherrypy globals.
-        cherrypy.tree = cherrypy._cptree.Tree()
-        cherrypy.config.reset()
-        setConfig(conf)
-        
-        m = __import__(testmod, globals(), locals())
-        setup = getattr(m, "setup_server", None)
-        if setup:
-            setup()
-        
-        # The setup functions probably mounted new apps.
-        # Tell our server about them.
-        sync_apps(profile=conf.get("profiling.on", False),
-                  validate=conf.get("validator.on", False),
-                  conquer=conf.get("conquer.on", False),
-                  )
-        
-        suite = CPTestLoader.loadTestsFromName(testmod)
-        result = CPTestRunner.run(suite)
-        cherrypy.engine.test_success &= result.wasSuccessful()
-        
-        teardown = getattr(m, "teardown_server", None)
-        if teardown:
-            teardown()
-    cherrypy.engine.stop()
+    try:
+        for testmod in moduleNames:
+            # Must run each module in a separate suite,
+            # because each module uses/overwrites cherrypy globals.
+            cherrypy.tree = cherrypy._cptree.Tree()
+            cherrypy.config.reset()
+            setConfig(conf)
+            
+            m = __import__(testmod, globals(), locals())
+            setup = getattr(m, "setup_server", None)
+            if setup:
+                setup()
+            
+            # The setup functions probably mounted new apps.
+            # Tell our server about them.
+            sync_apps(profile=conf.get("profiling.on", False),
+                      validate=conf.get("validator.on", False),
+                      conquer=conf.get("conquer.on", False),
+                      )
+            
+            suite = CPTestLoader.loadTestsFromName(testmod)
+            result = CPTestRunner.run(suite)
+            cherrypy.engine.test_success &= result.wasSuccessful()
+            
+            teardown = getattr(m, "teardown_server", None)
+            if teardown:
+                teardown()
+    finally:
+        cherrypy.engine.exit()
 
 def testmain(conf=None):
     """Run __main__ as a test module, with webtest debugging."""
         webtest.WebCase.PORT = cherrypy.server.socket_port
         webtest.main()
     finally:
-        cherrypy.engine.stop()
+        cherrypy.engine.exit()
 

cherrypy/test/test_states.py

             self.getPage("/")
             self.assertStatus(503)
         
-        # Block the main thread now and verify that stop() works.
-        def stoptest():
+        # Block the main thread now and verify that exit() works.
+        def exittest():
             self.getPage("/")
             self.assertBody("Hello World")
-            engine.stop()
+            engine.exit()
         cherrypy.server.start()
-        engine.start_with_callback(stoptest)
+        engine.start_with_callback(exittest)
         engine.block()
-        self.assertEqual(engine.state, engine.states.STOPPED)
+        self.assertEqual(engine.state, engine.states.EXITING)
     
     def test_1_Restart(self):
         cherrypy.server.start()
                 
                 self.assertEqual(db_connection.running, False)
                 self.assertEqual(len(db_connection.threads), 0)
-                self.assertEqual(engine.state, engine.states.STOPPED)
+                self.assertEqual(engine.state, engine.states.EXITING)
             finally:
                 self.persistent = False
             
             self.assertStatus(500)
             self.assertInBody("raise cherrypy.TimeoutError()")
         finally:
-            engine.stop()
+            engine.exit()
     
     def test_4_Autoreload(self):
         if not self.server_class:
             self.assert_(float(self.body) > start)
         finally:
             # Shut down the spawned process
-            self.getPage("/stop")
+            self.getPage("/exit")
         
         try:
             try:
             self.assertEqual(page_pid, pid)
         finally:
             # Shut down the spawned process
-            self.getPage("/stop")
+            self.getPage("/exit")
         
         try:
             print os.waitpid(pid, 0)
                 tr.stop()
                 tr.out.close()
     finally:
-        engine.stop()
+        engine.exit()
 
 
 def run_all(host, port, ssl=False):

cherrypy/test/test_states_demo.py

         return repr(starttime)
     start.exposed = True
     
-    def stop(self):
+    def exit(self):
         # This handler might be called before the engine is STARTED if an
         # HTTP worker thread handles it before the HTTP server returns
         # control to engine.start. We avoid that race condition here
         # by waiting for the Bus to be STARTED.
-        cherrypy.engine.block(state=cherrypy.engine.states.STARTED)
-        cherrypy.engine.stop()
-    stop.exposed = True
+        cherrypy.engine.wait(state=cherrypy.engine.states.STARTED)
+        cherrypy.engine.exit()
+    exit.exposed = True
 
 
 if __name__ == '__main__':
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.