Merge pull request #332 from CanonicalLtd/mwhudson/run_in_bg-callback

run_in_bg tweaks

subiquity/controllers/installprogress.py

@@ -76,7 +76,7 @@ class InstallProgressController(BaseController):
         self.progress_view.show_complete(True)
         self.default()
 
-    def run_command_logged(self, cmd, env):
+    def _bg_run_command_logged(self, cmd, env):
         log.debug("running %s", cmd)
         cmd = ['systemd-cat', '--level-prefix=false', '--identifier=' + self._log_syslog_identifier] + cmd
         cp = subprocess.run(cmd, env=env)
@@ -171,7 +171,7 @@ class InstallProgressController(BaseController):
         if 'SNAP' in env:
             del env['SNAP']
         self.run_in_bg(
-            lambda: self.run_command_logged(curtin_cmd, env),
+            lambda: self._bg_run_command_logged(curtin_cmd, env),
             self.curtin_install_completed)
 
     def curtin_install_completed(self, fut):
@@ -182,12 +182,6 @@ class InstallProgressController(BaseController):
             return
         self.install_state = InstallState.DONE
         log.debug('After curtin install OK')
-        self.loop.set_alarm_in(0.01, lambda loop, userdata: self.install_complete())
-
-    def cancel(self):
-        pass
-
-    def install_complete(self):
         self.ui.progress_current += 1
         if not self.progress_view_showing:
             self.ui.set_footer(_("Install complete"))
@@ -197,6 +191,9 @@ class InstallProgressController(BaseController):
         if self._identity_config_done:
             self.postinstall_configuration()
 
+    def cancel(self):
+        pass
+
     def postinstall_configuration(self):
         # If we need to do anything that takes time here (like running
         # dpkg-reconfigure maas-rack-controller, for example...) we
@@ -209,7 +206,7 @@ class InstallProgressController(BaseController):
         self.progress_view.show_complete()
 
         if self.answers['reboot']:
-            self.loop.set_alarm_in(0.01, lambda loop, userdata: self.reboot())
+            self.reboot()
 
     def configure_cloud_init(self):
         if self.opts.dry_run:

subiquity/controllers/keyboard.py

@@ -66,7 +66,7 @@ class KeyboardController(BaseController):
             self._done)
 
     def _done(self, fut):
-        self.loop.set_alarm_in(0.0, lambda loop, ud: self.signal.emit_signal('next-screen'))
+        self.signal.emit_signal('next-screen')
 
     def cancel(self):
         self.signal.emit_signal('prev-screen')

subiquitycore/controller.py

@@ -49,15 +49,12 @@ class BaseController(ABC):
         """Run func() in a thread and call callback on UI thread.
 
         callback will be passed a concurrent.futures.Future containing
-        the result of func(). The result of callback is discarded. Any
-        exception will be logged.
+        the result of func(). The result of callback is discarded. An
+        exception will crash the process so be careful!
         """
         fut = self.pool.submit(func)
         def in_main_thread(ignored):
-            try:
             callback(fut)
-            except:
-                log.exception("callback %s after calling %s failed", callback, func)
         pipe = self.loop.watch_pipe(in_main_thread)
         def in_random_thread(ignored):
             os.write(pipe, b'x')

subiquitycore/controllers/network.py

@@ -51,7 +51,7 @@ class BackgroundTask:
         """
         raise NotImplementedError(self.start)
 
-    def run(self):
+    def _bg_run(self):
         """Run the task.
 
         This is called on an arbitrary thread so don't do UI stuff!
@@ -89,7 +89,7 @@ class BackgroundProcess(BackgroundTask):
     def start(self):
         self.proc = run_command_start(self.cmd)
 
-    def run(self):
+    def _bg_run(self):
         stdout, stderr = self.proc.communicate()
         return run_command_summarize(self.proc, stdout, stderr)
 
@@ -121,7 +121,7 @@ class PythonSleep(BackgroundTask):
     def start(self):
         pass
 
-    def run(self):
+    def _bg_run(self):
         r, _, _ = select.select([self.r], [], [], self.duration)
         if not r:
             return True
@@ -156,7 +156,7 @@ class DownNetworkDevices(BackgroundTask):
                 # We don't actually care very much about this
                 log.exception('unset_link_flags failed for %s', dev.name)
 
-    def run(self):
+    def _bg_run(self):
         return True
 
     def end(self, observer, fut):
@@ -186,7 +186,7 @@ class WaitForDefaultRouteTask(BackgroundTask):
         self.success_r, self.success_w = os.pipe()
         self.event_receiver.add_default_route_waiter(self.got_route)
 
-    def run(self):
+    def _bg_run(self):
         try:
             r, _, _ = select.select([self.fail_r, self.success_r], [], [], self.timeout)
             return self.success_r in r
@@ -229,7 +229,7 @@ class TaskSequence:
         self.tasks = self.tasks[1:]
         log.debug('running %s for stage %s', self.curtask, self.stage)
         self.curtask.start()
-        self.run_in_bg(self.curtask.run, lambda fut:self.curtask.end(self, fut))
+        self.run_in_bg(self.curtask._bg_run, lambda fut:self.curtask.end(self, fut))
 
     def task_succeeded(self):
         if self.canceled:
@@ -445,7 +445,7 @@ class NetworkController(BaseController):
 
     def tasks_finished(self):
         self.signal.emit_signal('network-config-written', self.netplan_path)
-        self.loop.set_alarm_in(0.0, lambda loop, ud: self.signal.emit_signal('next-screen'))
+        self.signal.emit_signal('next-screen')
 
     def set_default_v4_route(self):
         self.ui.set_header("Default route")

subiquitycore/core.py

@@ -261,7 +261,7 @@ class Application:
             "signal": Signal(),
             "prober": prober,
             "loop": None,
-            "pool": futures.ThreadPoolExecutor(1),
+            "pool": futures.ThreadPoolExecutor(4),
             "answers": answers,
             "input_filter": input_filter,
         }