Fix system.multicall() broken by faster start/stop patch

In past versions, startProcess() and stopProcess() would always return
a callback.  50d18577af28b6b053c0d446c959b42b5300a0f8 changed this
so they may return either a callback or a bool, but the code that handles
system.multicall() was not updated.  If multicall was used with stopProcess()
followed by startProcess(), it would try to start the process before it had
finished stopping.  This broke the restart process link on the web interface.

supervisor/tests/test_rpcinterfaces.py

@@ -2163,44 +2163,34 @@ class SystemNamespaceXMLRPCInterfaceTests(TestBase):
 
     def test_multicall_simplevals(self):
         interface = self._makeOne()
-        callback = interface.multicall([
+        results = interface.multicall([
             {'methodName':'system.methodHelp', 'params':['system.methodHelp']},
             {'methodName':'system.listMethods', 'params':[]},
             ])
-        from supervisor import http
-        result = http.NOT_DONE_YET
-        while result is http.NOT_DONE_YET:
-            result = callback()
-        self.assertEqual(result[0], interface.methodHelp('system.methodHelp'))
-        self.assertEqual(result[1], interface.listMethods())
+        self.assertEqual(results[0], interface.methodHelp('system.methodHelp'))
+        self.assertEqual(results[1], interface.listMethods())
 
     def test_multicall_recursion_guard(self):
         from supervisor import xmlrpc
         interface = self._makeOne()
-        callback = interface.multicall([
+        results = interface.multicall([
             {'methodName': 'system.multicall', 'params': []},
         ])
 
-        from supervisor import http
-        result = http.NOT_DONE_YET
-        while result is http.NOT_DONE_YET:
-            result = callback()
-
-        code = xmlrpc.Faults.INCORRECT_PARAMETERS
-        desc = xmlrpc.getFaultDescription(code)
-        recursion_fault = {'faultCode': code, 'faultString': desc}
-
-        self.assertEqual(result, [recursion_fault])
+        e = xmlrpc.RPCError(xmlrpc.Faults.INCORRECT_PARAMETERS,
+                'Recursive system.multicall forbidden')
+        recursion_fault = {'faultCode': e.code, 'faultString': e.text}
+        self.assertEqual(results, [recursion_fault])
 
     def test_multicall_nested_callback(self):
+        from supervisor import http
         interface = self._makeOne()
         callback = interface.multicall([
             {'methodName':'supervisor.stopAllProcesses'}])
-        from supervisor import http
-        result = http.NOT_DONE_YET
-        while result is http.NOT_DONE_YET:
-            result = callback()
-        self.assertEqual(result[0], [])
+        results = http.NOT_DONE_YET
+        while results is http.NOT_DONE_YET:
+            results = callback()
+        self.assertEqual(results[0], [])
 
     def test_methodHelp(self):
         from supervisor import xmlrpc

supervisor/tests/test_xmlrpc.py

@@ -569,86 +569,152 @@ class TestSystemNamespaceRPCInterface(unittest.TestCase):
         inst = self._makeOne([('ns1', ns1)])
         self.assertRaises(RPCError, inst.methodSignature, 'ns1.foo')
 
-    def test_multicall_recursion_forbidden(self):
+    def test_multicall_faults_for_recursion(self):
+        from supervisor.xmlrpc import Faults
         inst = self._makeOne()
-        call = {'methodName':'system.multicall'}
-        multiproduce = inst.multicall([call])
-        result = multiproduce()
+        calls = [{'methodName':'system.multicall'}]
+        results = inst.multicall(calls)
         self.assertEqual(
-            result,
-            [{'faultCode': 2, 'faultString': 'INCORRECT_PARAMETERS'}]
+            results,
+            [{'faultCode': Faults.INCORRECT_PARAMETERS,
+              'faultString': ('INCORRECT_PARAMETERS: Recursive '
+                              'system.multicall forbidden')}]
             )
 
-    def test_multicall_other_exception(self):
+    def test_multicall_faults_for_missing_methodName(self):
+        from supervisor.xmlrpc import Faults
         inst = self._makeOne()
-        call = {} # no methodName
-        multiproduce = inst.multicall([call])
-        result = multiproduce()
-        self.assertEqual(len(result), 1)
-        self.assertEqual(result[0]['faultCode'], 1)
+        calls = [{}]
+        results = inst.multicall(calls)
+        self.assertEqual(
+            results,
+            [{'faultCode': Faults.INCORRECT_PARAMETERS,
+              'faultString': 'INCORRECT_PARAMETERS: No methodName'}]
+            )
 
-    def test_multicall_no_calls(self):
+    def test_multicall_faults_for_methodName_bad_namespace(self):
+        from supervisor.xmlrpc import Faults
         inst = self._makeOne()
-        multiproduce = inst.multicall([])
-        result = multiproduce()
-        self.assertEqual(result, [])
+        calls = [{'methodName': 'bad.stopProcess'}]
+        results = inst.multicall(calls)
+        self.assertEqual(
+            results,
+            [{'faultCode': Faults.UNKNOWN_METHOD,
+              'faultString': 'UNKNOWN_METHOD'}]
+            )
 
-    def test_multicall_callback_raises_RPCError(self):
-        from supervisor.xmlrpc import RPCError, Faults
+    def test_multicall_faults_for_methodName_good_ns_bad_method(self):
+        from supervisor.xmlrpc import Faults
         class DummyNamespace(object):
-            def foo(self):
-                """ @param string name The thing"""
-                raise RPCError(Faults.UNKNOWN_METHOD)
-
+            pass
         ns1 = DummyNamespace()
         inst = self._makeOne([('ns1', ns1)])
-        multiproduce = inst.multicall([{'methodName':'ns1.foo'}])
-        result = multiproduce()
+        calls = [{'methodName': 'ns1.bad'}]
+        results = inst.multicall(calls)
         self.assertEqual(
-            result, [{'faultString': 'UNKNOWN_METHOD', 'faultCode': 1}]
+            results,
+            [{'faultCode': Faults.UNKNOWN_METHOD,
+              'faultString': 'UNKNOWN_METHOD'}]
             )
 
-    def test_multicall_callback_returns_function_returns_NOT_DONE_YET(self):
+    def test_multicall_returns_empty_results_for_empty_calls(self):
+        inst = self._makeOne()
+        calls = []
+        results = inst.multicall(calls)
+        self.assertEqual(results, [])
+
+    def test_multicall_performs_noncallback_functions_serially(self):
+        class DummyNamespace(object):
+            def say(self, name):
+                """ @param string name Process name"""
+                return name
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [
+            {'methodName': 'ns1.say', 'params': ['Alvin']},
+            {'methodName': 'ns1.say', 'params': ['Simon']},
+            {'methodName': 'ns1.say', 'params': ['Theodore']}
+        ]
+        results = inst.multicall(calls)
+        self.assertEqual(results, ['Alvin', 'Simon', 'Theodore'])
+
+    def test_multicall_catches_noncallback_exceptions(self):
+        import errno
+        from supervisor.xmlrpc import RPCError, Faults
+        class DummyNamespace(object):
+            def bad_name(self):
+                raise RPCError(Faults.BAD_NAME, 'foo')
+            def os_error(self):
+                raise OSError(errno.ENOENT)
+        ns1 = DummyNamespace()
+        inst = self._makeOne([('ns1', ns1)])
+        calls = [{'methodName': 'ns1.bad_name'}, {'methodName': 'ns1.os_error'}]
+        results = inst.multicall(calls)
+
+        bad_name = {'faultCode': Faults.BAD_NAME,
+                    'faultString': 'BAD_NAME: foo'}
+        os_error = {'faultCode': Faults.FAILED,
+                    'faultString': "FAILED: %s:2" % OSError}
+        self.assertEqual(results, [bad_name, os_error])
+
+    def test_multicall_catches_callback_exceptions(self):
+        import errno
+        from supervisor.xmlrpc import RPCError, Faults
         from supervisor.http import NOT_DONE_YET
         class DummyNamespace(object):
-            def __init__(self):
-                self.results = [NOT_DONE_YET, 1]
-            def foo(self):
-                """ @param string name The thing"""
+            def bad_name(self):
+                def inner():
+                    raise RPCError(Faults.BAD_NAME, 'foo')
+                return inner
+            def os_error(self):
                 def inner():
-                    return self.results.pop(0)
+                    raise OSError(errno.ENOENT)
                 return inner
         ns1 = DummyNamespace()
         inst = self._makeOne([('ns1', ns1)])
-        multiproduce = inst.multicall([{'methodName':'ns1.foo'}])
-        result = multiproduce()
-        self.assertEqual(
-            result,
-            NOT_DONE_YET
-            )
-        result = multiproduce()
-        self.assertEqual(
-            result,
-            [1]
-            )
-
-    def test_multicall_callback_returns_function_raises_RPCError(self):
-        from supervisor.xmlrpc import Faults, RPCError
+        calls = [{'methodName': 'ns1.bad_name'}, {'methodName': 'ns1.os_error'}]
+        callback = inst.multicall(calls)
+        results = NOT_DONE_YET
+        while results is NOT_DONE_YET:
+            results = callback()
+
+        bad_name = {'faultCode': Faults.BAD_NAME,
+                    'faultString': 'BAD_NAME: foo'}
+        os_error = {'faultCode': Faults.FAILED,
+                    'faultString': "FAILED: %s:2" % OSError}
+        self.assertEqual(results, [bad_name, os_error])
+
+    def test_multicall_performs_callback_functions_serially(self):
+        from supervisor.http import NOT_DONE_YET
         class DummyNamespace(object):
-            def foo(self):
-                """ @param string name The thing"""
+            def __init__(self):
+                self.stop_results = [NOT_DONE_YET, NOT_DONE_YET,
+                    NOT_DONE_YET, 'stop result']
+                self.start_results = ['start result']
+            def stopProcess(self, name):
+                def inner():
+                    result = self.stop_results.pop(0)
+                    if result is not NOT_DONE_YET:
+                        self.stopped = True
+                    return result
+                return inner
+            def startProcess(self, name):
                 def inner():
-                    raise RPCError(Faults.UNKNOWN_METHOD)
+                    if not self.stopped:
+                        raise Exception("This should not raise")
+                    return self.start_results.pop(0)
                 return inner
         ns1 = DummyNamespace()
         inst = self._makeOne([('ns1', ns1)])
-        multiproduce = inst.multicall([{'methodName':'ns1.foo'}])
-        result = multiproduce()
-        self.assertEqual(
-            result,
-            [{'faultString': 'UNKNOWN_METHOD', 'faultCode': 1}],
-            )
-
+        calls = [{'methodName': 'ns1.stopProcess',
+                  'params': {'name': 'foo'}},
+                 {'methodName': 'ns1.startProcess',
+                  'params': {'name': 'foo'}}]
+        callback = inst.multicall(calls)
+        results = NOT_DONE_YET
+        while results is NOT_DONE_YET:
+            results = callback()
+        self.assertEqual(results, ['stop result', 'start result'])
 
 class Test_gettags(unittest.TestCase):
     def _callFUT(self, comment):

supervisor/web.py

@@ -414,20 +414,27 @@ class StatusView(MeldView):
                         return stopdone
 
                 elif action == 'restart':
-                    callback = rpcinterface.system.multicall(
+                    results_or_callback = rpcinterface.system.multicall(
                         [ {'methodName':'supervisor.stopProcess',
                            'params': [namespec]},
                           {'methodName':'supervisor.startProcess',
                            'params': [namespec]},
                           ]
                         )
-                    def restartprocess():
-                        result = callback()
-                        if result is NOT_DONE_YET:
-                            return NOT_DONE_YET
-                        return 'Process %s restarted' % namespec
-                    restartprocess.delay = 0.05
-                    return restartprocess
+                    if callable(results_or_callback):
+                        callback = results_or_callback
+                        def restartprocess():
+                            results = callback()
+                            if results is NOT_DONE_YET:
+                                return NOT_DONE_YET
+                            return 'Process %s restarted' % namespec
+                        restartprocess.delay = 0.05
+                        return restartprocess
+                    else:
+                        def restartdone():
+                            return 'Process %s restarted' % namespec
+                        restartdone.delay = 0.05
+                        return restartdone
 
                 elif action == 'clearlog':
                     try:

supervisor/xmlrpc.py

@@ -233,63 +233,81 @@ class SystemNamespaceRPCInterface:
         @param array calls  An array of call requests
         @return array result  An array of results
         """
-        producers = []
+        remaining_calls = calls[:] # [{'methodName':x, 'params':x}, ...]
+        callbacks = [] # always empty or 1 callback function only
+        results = [] # results of completed calls
 
-        for call in calls:
-            try:
-                name = call['methodName']
-                params = call.get('params', [])
-                if name == 'system.multicall':
-                    # Recursive system.multicall forbidden
-                    raise RPCError(Faults.INCORRECT_PARAMETERS)
-                root = AttrDict(self.namespaces)
-                value = traverse(root, name, params)
-            except RPCError as inst:
-                value = {'faultCode': inst.code,
-                         'faultString': inst.text}
-            except:
-                errmsg = "%s:%s" % (sys.exc_info()[0], sys.exc_info()[1])
-                value = {'faultCode': 1, 'faultString': errmsg}
-            producers.append(value)
-
-        results = []
-
-        def multiproduce():
-            """ Run through all the producers in order """
-            if not producers:
-                return []
-
-            callback = producers.pop(0)
-
-            if isinstance(callback, types.FunctionType):
-                try:
-                    value = callback()
-                except RPCError as inst:
-                    value = {'faultCode':inst.code, 'faultString':inst.text}
-
-                if value is NOT_DONE_YET:
-                    # push it back in the front of the queue because we
-                    # need to finish the calls in requested order
-                    producers.insert(0, callback)
-                    return NOT_DONE_YET
-            else:
-                value = callback
+        # args are only to fool scoping and are never passed by caller
+        def multi(remaining_calls=remaining_calls,
+                  callbacks=callbacks,
+                  results=results):
 
-            results.append(value)
+            # if waiting on a callback, call it, then remove it if it's done
+            if callbacks:
+                try:
+                    value = callbacks[0]()
+                except RPCError as exc:
+                    value = {'faultCode': exc.code,
+                             'faultString': exc.text}
+                except:
+                    info = sys.exc_info()
+                    errmsg = "%s:%s" % (info[0], info[1])
+                    value = {'faultCode': Faults.FAILED,
+                             'faultString': 'FAILED: ' + errmsg}
+                if value is not NOT_DONE_YET:
+                    callbacks.pop(0)
+                    results.append(value)
+
+            # if we don't have a callback now, pop calls and call them in
+            # order until one returns a callback.
+            while (not callbacks) and remaining_calls:
+                call = remaining_calls.pop(0)
+                name = call.get('methodName', None)
+                params = call.get('params', [])
 
-            if producers:
-                # only finish when all producers are finished
+                try:
+                    if name is None:
+                        raise RPCError(Faults.INCORRECT_PARAMETERS,
+                            'No methodName')
+                    if name == 'system.multicall':
+                        raise RPCError(Faults.INCORRECT_PARAMETERS,
+                            'Recursive system.multicall forbidden')
+                    # make the call, may return a callback or not
+                    root = AttrDict(self.namespaces)
+                    value = traverse(root, name, params)
+                except RPCError as exc:
+                    value = {'faultCode': exc.code,
+                             'faultString': exc.text}
+                except:
+                    info = sys.exc_info()
+                    errmsg = "%s:%s" % (info[0], info[1])
+                    value = {'faultCode': Faults.FAILED,
+                             'faultString': 'FAILED: ' + errmsg}
+
+                if isinstance(value, types.FunctionType):
+                    callbacks.append(value)
+                else:
+                    results.append(value)
+
+            # we are done when there's no callback and no more calls queued
+            if callbacks or remaining_calls:
                 return NOT_DONE_YET
-
-            return results
-
-        multiproduce.delay = .05
-        return multiproduce
+            else:
+                return results
+        multi.delay = 0.05
+
+        # optimization: multi() is called here instead of just returning
+        # multi in case all calls complete and we can return with no delay.
+        value = multi()
+        if value is NOT_DONE_YET:
+            return multi
+        else:
+            return value
 
 class AttrDict(dict):
     # hack to make a dict's getattr equivalent to its getitem
     def __getattr__(self, name):
-        return self[name]
+        return self.get(name)
 
 class RootRPCInterface:
     def __init__(self, subinterfaces):