clean up thread pools and fix initial_reset race condition

malcolm/controllers/runnablecontroller.py

@@ -324,8 +324,8 @@ def update_completed_steps(self, completed_steps, part):
             self.completed_steps.set_value(min_completed_steps)
 
     @method_writeable_in(
-        sm.IDLE, sm.CONFIGURING, sm.READY, sm.RUNNING, sm.POSTRUN, sm.RESETTING,
-        sm.PAUSED, sm.SEEKING)
+        sm.IDLE, sm.CONFIGURING, sm.READY, sm.RUNNING, sm.POSTRUN, sm.PAUSED,
+        sm.SEEKING)
     def abort(self):
         self.try_stateful_function(sm.ABORTING, sm.ABORTED, self.do_abort)
 

malcolm/core/process.py

@@ -96,6 +96,8 @@ def stop(self, timeout=None):
         # Now wait for anything it spawned to complete
         for s in self._other_spawned:
             s.wait(timeout=timeout)
+        # Garbage collect the syncfactory
+        del self.sync_factory
 
     def _forward_block_request(self, request):
         """Lookup target Block and spawn block.handle_request(request)

malcolm/core/statemachine.py

@@ -153,7 +153,7 @@ def create_states(self):
         # Add Abort to all normal states
         normal_states = [
             self.IDLE, self.READY, self.CONFIGURING, self.RUNNING, self.POSTRUN,
-            self.PAUSED, self.RESETTING, self.SEEKING]
+            self.PAUSED, self.SEEKING]
         for state in normal_states:
             self.set_allowed(state, self.ABORTING)
 

malcolm/parts/builtin/runnablechildpart.py

@@ -19,12 +19,16 @@ def update_configure_validate_args(self):
     @RunnableController.Reset
     def reset(self, task):
         # Wait until we are Idle
-        if self.child.state == sm.RESETTING:
-            task.when_matches(self.child["state"], sm.IDLE)
-        else:
-            if self.child["abort"].writeable:
-                task.post(self.child["abort"])
+        if self.child["abort"].writeable:
+            task.post(self.child["abort"])
+        try:
             task.post(self.child["reset"])
+        except ValueError:
+            # We get a "ValueError: child is not writeable" if we can't run
+            # reset, probably because the child is already resetting,
+            # so just wait for it to be idle
+            task.when_matches(
+                self.child["state"], sm.IDLE, bad_values=[sm.FAULT])
         self.update_configure_validate_args()
 
     @RunnableController.Validate

tests/test_comms/test_websocket/test_system_websocket.py

@@ -28,8 +28,7 @@ class TestSystemWSCommsServerOnly(unittest.TestCase):
     socket = 8881
 
     def setUp(self):
-        self.sf = SyncFactory("sync")
-        self.process = Process("proc", self.sf)
+        self.process = Process("proc", SyncFactory("sync"))
         Hello(self.process, dict(mri="hello"))
         self.process.add_comms(
             WebsocketServerComms(self.process, dict(port=self.socket)))
@@ -69,8 +68,8 @@ class TestSystemWSCommsServerAndClient(unittest.TestCase):
     socket = 8882
 
     def setUp(self):
-        self.sf = SyncFactory("sync")
-        self.process = Process("proc", self.sf)
+        sf = SyncFactory("sync")
+        self.process = Process("proc", sf)
         Hello(self.process, dict(mri="hello"))
         Counter(self.process, dict(mri="counter"))
         self.process.add_comms(
@@ -79,7 +78,7 @@ def setUp(self):
         # If we don't wait long enough, sometimes the websocket_connect()
         # in process2 will hang...
         time.sleep(0.1)
-        self.process2 = Process("proc2", self.sf)
+        self.process2 = Process("proc2", sf)
         self.process2.add_comms(
             WebsocketClientComms(self.process2,
                              dict(hostname="localhost", port=self.socket)))
@@ -88,7 +87,9 @@ def setUp(self):
     def tearDown(self):
         self.socket += 1
         self.process.stop()
+        del self.process
         self.process2.stop()
+        del self.process2
 
     def test_server_hello_with_malcolm_client(self):
         block2 = self.process2.make_client_block("hello")

tests/test_controllers/test_managercontroller.py

@@ -14,7 +14,7 @@
 # module imports
 from malcolm.controllers.managercontroller import ManagerController
 from malcolm.core import method_writeable_in, method_takes, DefaultStateMachine
-from malcolm.core import Process, Part, Table
+from malcolm.core import Process, Part, Table, Task
 from malcolm.core.syncfactory import SyncFactory
 from malcolm.parts.builtin.childpart import ChildPart
 
@@ -28,8 +28,7 @@ def checkState(self, state, child=True, parent=True):
             self.assertEqual(self.c.state.value, state)
 
     def setUp(self):
-        self.s = SyncFactory('threading')
-        self.p = Process('process1', self.s)
+        self.p = Process('process1', SyncFactory('threading'))
 
         # create a child ManagerController block
         params = ManagerController.MethodMeta.\
@@ -56,12 +55,15 @@ def setUp(self):
         self.checkState(self.sm.DISABLED)
         self.p.start()
 
-        retry = 0
-        while retry < 20 and self.c.state.value != self.sm.READY:
-            sleep(.1)
-            retry += 1
+        # wait until block is Ready
+        task = Task("block_ready_task", self.p)
+        task.when_matches(self.b["state"], self.sm.READY, timeout=1)
+
         self.checkState(self.sm.READY)
 
+    def tearDown(self):
+        self.p.stop()
+
     def test_init(self):
 
         # the following block attributes should be created by a call to

tests/test_controllers/test_runnablecontroller.py

@@ -15,7 +15,7 @@
 from malcolm.core import method_writeable_in, method_takes, \
         DefaultStateMachine, Block, Controller, REQUIRED
 from malcolm.core.vmetas import BooleanMeta
-from malcolm.core import Process, Part, RunnableStateMachine
+from malcolm.core import Process, Part, RunnableStateMachine, Task
 from malcolm.core.syncfactory import SyncFactory
 from malcolm.controllers.runnablecontroller import RunnableController
 from scanpointgenerator import LineGenerator, CompoundGenerator
@@ -40,8 +40,7 @@ def checkSteps(self, configured, completed, total):
     def setUp(self):
         self.maxDiff = 5000
 
-        self.s = SyncFactory('threading')
-        self.p = Process('process1', self.s)
+        self.p = Process('process1', SyncFactory('threading'))
 
         # create a child RunnableController block
         params = RunnableController.MethodMeta.prepare_input_map(
@@ -68,13 +67,14 @@ def setUp(self):
         self.checkState(self.sm.DISABLED)
         self.p.start()
 
-        retry = 0
-        while  retry < 20 and self.c.state.value != self.sm.IDLE:
-            sleep(.5)
-            retry += 1
-        self.checkState(self.sm.IDLE)
+        # wait until block is Ready
+        task = Task("block_ready_task", self.p)
+        task.when_matches(self.b["state"], self.sm.IDLE, timeout=1)
 
+        self.checkState(self.sm.IDLE)
 
+    def tearDown(self):
+        self.p.stop()
 
     def test_init(self):
         # the following block attributes should be created by a call to

tests/test_core/test_statemachine.py

@@ -106,7 +106,7 @@ def setUp(self):
     def test_init(self):
         default_allowed_transitions = OrderedDict()
         default_allowed_transitions['Resetting'] = {
-            "Idle", "Aborting", "Fault", "Disabling"}
+            "Idle", "Fault", "Disabling"}
         default_allowed_transitions['Idle'] = {
             "Configuring", "Aborting", 'Editing', "Fault", "Disabling"}
         default_allowed_transitions['Editing'] = {