Merge pull request #442 from jacob-lee/shutdownfix

Refactor server shutdown

psiturk/experiment.py

@@ -21,6 +21,7 @@
 import json
 from jinja2 import TemplateNotFound
 from collections import Counter
+import signal
 
 # Setup flask
 from flask import Flask, render_template, render_template_string, request, \
@@ -59,6 +60,14 @@
 # Let's start
 # ===========
 
+# Unfortunately, this does not tell psiturk interactive to update server status
+def sigint_handler(signal, frame):
+    print('^C: shutting down server processes.')
+    sys.exit(0)
+
+
+signal.signal(signal.SIGINT, sigint_handler)
+
 app = Flask("Experiment_Server")
 
 # experiment server logging

psiturk/experiment_server.py

@@ -64,17 +64,6 @@ def load_user_config(self):
 
         self.loglevels = ["debug", "info", "warning", "error", "critical"]
 
-        def on_exit(server):
-            '''
-            this is hooked so that it can be called when
-            the server is shut down via CTRL+C. Otherwise
-            there is no notification to the user that the server
-            has shut down until they hit `enter` and see that
-            the cmdloop prompt suddenly says "server off"
-            '''
-            print('Caught ^C, experiment server has shut down.')
-            print('Press `enter` to continue.')
-
         # add unique identifier of this psiturk project folder
         project_hash = hashlib.sha1(os.getcwd().encode()).hexdigest()[:12]
         self.user_options = {
@@ -87,7 +76,6 @@ def on_exit(server):
             'errorlog': config.get("Server Parameters", "logfile"),
             'proc_name': 'psiturk_experiment_server_' + project_hash,
             'limit_request_line': '0',
-            'on_exit': on_exit
         }
 
         if config.has_option("Server Parameters", "certfile") and config.has_option("Server Parameters", "keyfile"):

psiturk/experiment_server_controller.py

@@ -8,14 +8,21 @@
 import socket
 from threading import Thread, Event
 import webbrowser
-import signal
 import subprocess
 import sys
 import os
+import logging
 from builtins import object
 from future import standard_library
 standard_library.install_aliases()
 
+stream_handler = logging.StreamHandler(sys.stdout)
+stream_handler.setLevel(logging.INFO)  # TODO: let this be configurable
+stream_formatter = logging.Formatter('%(message)s')
+stream_handler.setFormatter(stream_formatter)
+logger = logging.getLogger(__name__)
+logger.addHandler(stream_handler)
+logger.setLevel(logging.DEBUG)
 
 #!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 # Supporting functions
@@ -29,7 +36,8 @@ def is_port_available(ip, port):
         s.shutdown(2)
         return False
     except socket.timeout:
-        print("*** Failed to test port availability. Check that host\nis set properly in config.txt")
+        logger.error("*** Failed to test port availability. "
+                     "Check that host is set properly in config.txt")
         return True
     except socket.error as e:
         return True
@@ -113,41 +121,64 @@ def __init__(self, config):
         self.config = config
         self.server_running = False
 
-    def get_ppid(self):
-        if not self.is_port_available():
-            url = "http://{hostname}:{port}/ppid".format(hostname=self.config.get(
-                "Server Parameters", "host"), port=self.config.getint("Server Parameters", "port"))
-            ppid_request = urllib.request.Request(url)
-            ppid = urllib.request.urlopen(ppid_request).read()
-            return ppid
-        else:
-            raise ExperimentServerControllerException(
-                "Cannot shut down experiment server, server not online")
-
     def restart(self):
         self.shutdown()
         self.startup()
 
+    def on_terminate(self, proc: psutil.Process):
+        logger.debug("process {} terminated with exit code {}".format(
+            proc, proc.returncode))
+
+    def kill_process_tree(self, proc: psutil.Process):
+        """Kill process tree with given process object.
+
+        Caller should be prepared to catch psutil Process class exceptions.
+        """
+        children = proc.children(recursive=True)
+        children.append(proc)
+        for c in children:
+            c.terminate()
+        gone, alive = psutil.wait_procs(children, timeout=10, callback=self.on_terminate)
+        for survivor in alive:
+            survivor.kill()
+
     def shutdown(self, ppid=None):
-        if not ppid:
-            ppid = self.get_ppid()
-        print("Shutting down experiment server at pid %s..." % ppid)
-        try:
-            os.kill(int(ppid), signal.SIGKILL)
-            self.server_running = False
-        except ExperimentServerControllerException:
-            print(ExperimentServerControllerException)
-        else:
+        proc_hash = self.get_project_hash()
+        psiturk_master_procs = [p for p in psutil.process_iter(
+            ['pid', 'cmdline', 'exe', 'name']) if proc_hash in str(p.info) and
+                               'master' in str(p.info)]
+        if len(psiturk_master_procs) < 1:
+            logger.warning('No active server process found.')
             self.server_running = False
+            return
+        for p in psiturk_master_procs:
+            logger.info('Shutting down experiment server at pid %s ... ' % p.info['pid'])
+            try:
+                self.kill_process_tree(p)
+            except psutil.NoSuchProcess as e:
+                logger.error('Attempt to shut down PID {} failed with exception {}'.format(
+                    p.as_dict['pid'], e
+                ))
+        # NoSuchProcess exceptions imply server is not running, so seems safe.
+        self.server_running = False
 
-    def is_server_running(self):
-        project_hash = hashlib.sha1(os.getcwd().encode()).hexdigest()[:12]
-        proc_name = "psiturk_experiment_server_" + project_hash
+    def check_server_process_running(self, process_hash):
         server_process_running = False
         for proc in psutil.process_iter():
-            if proc_name in str(proc.as_dict(['cmdline'])):
+            if process_hash in str(proc.as_dict(['cmdline'])):
                 server_process_running = True
                 break
+        return server_process_running
+
+    def get_project_hash(self):
+        project_hash = 'psiturk_experiment_server_{}'.format(
+            hashlib.sha1(os.getcwd().encode()).hexdigest()[:12]
+        )
+        return project_hash
+
+    def is_server_running(self):
+        process_hash = self.get_project_hash()
+        server_process_running = self.check_server_process_running(process_hash)
         port_is_open = self.is_port_available()
         if port_is_open and server_process_running:  # This should never occur
             return 'maybe'
@@ -170,15 +201,14 @@ def startup(self):
         )
         server_status = self.is_server_running()
         if server_status == 'no':
-            #print "Running experiment server with command:", server_command
             subprocess.Popen(server_command, shell=True, close_fds=True)
-            print("Experiment server launching...")
+            logging.info("Experiment server launching...")
             self.server_running = True
         elif server_status == 'maybe':
-            print("Error: Not sure what to tell you...")
+            logging.error("Error: Not sure what to tell you...")
         elif server_status == 'yes':
-            print("Experiment server may be already running...")
+            logging.warning("Experiment server may be already running...")
         elif server_status == 'blocked':
-            print(
+            logging.warning(
                 "Another process is running on the desired port. Try using a different port number.")
         time.sleep(1.2)  # Allow CLI to catch up.