[U-Boot] [PATCH] test/py: handle exceptions in console creation

[Simon Glass]

Hi Stephen,

On 10 February 2016 at 16:54, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at nvidia.com>
>
> u_boot_console.exec_attach.get_spawn() performs two steps:
> 1) Spawn a process to communicate with the serial console.
> 2) Reset the board so that U-Boot starts running from scratch.
>
> Currently, if an exception happens in step (2), no cleanup is performed on
> the process created in step (1). That process stays running and may e.g.
> hold serial port locks, or simply continue to read data from the serial
> port, thus preventing it from reaching any other process that attempts to
> read from the same serial port later. While there is error cleanup code in
> u_boot_console_base.ensure_spawned(), this is not triggered since the
> exception prevents assignment to self.p there, and hence the exception
> handler has no object to operate upon in cleanup_spawn().
>
> Solve this by enhancing u_boot_console.exec_attach.get_spawn() to clean
> up any objects it has created.
>
> In theory, u_boot_spawn.Spawn's constructor has a similar issue, so fix
> this too.
>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
>  test/py/u_boot_console_exec_attach.py | 14 +++++++++-----
>  test/py/u_boot_spawn.py               |  8 ++++++--
>  2 files changed, 15 insertions(+), 7 deletions(-)
>

Acked-by: Simon Glass <sjg at chromium.org>

But please see below.

> diff --git a/test/py/u_boot_console_exec_attach.py b/test/py/u_boot_console_exec_attach.py
> index 1be27c193079..445b58dda612 100644
> --- a/test/py/u_boot_console_exec_attach.py
> +++ b/test/py/u_boot_console_exec_attach.py
> @@ -58,10 +58,14 @@ class ConsoleExecAttach(ConsoleBase):
>          args = [self.config.board_type, self.config.board_identity]
>          s = Spawn(['u-boot-test-console'] + args)
>
> -        self.log.action('Resetting board')
> -        cmd = ['u-boot-test-reset'] + args
> -        runner = self.log.get_runner(cmd[0], sys.stdout)
> -        runner.run(cmd)
> -        runner.close()
> +        try:
> +            self.log.action('Resetting board')
> +            cmd = ['u-boot-test-reset'] + args
> +            runner = self.log.get_runner(cmd[0], sys.stdout)
> +            runner.run(cmd)
> +            runner.close()
> +        except:
> +            s.close()
> +            raise

Would try...finally work here? It might avoid the 'raise'.

>
>          return s
> diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
> index 3d9cde5ee0d0..a5f4a8e91bae 100644
> --- a/test/py/u_boot_spawn.py
> +++ b/test/py/u_boot_spawn.py
> @@ -56,8 +56,12 @@ class Spawn(object):
>              finally:
>                  os._exit(255)
>
> -        self.poll = select.poll()
> -        self.poll.register(self.fd, select.POLLIN | select.POLLPRI | select.POLLERR | select.POLLHUP | select.POLLNVAL)
> +        try:
> +            self.poll = select.poll()
> +            self.poll.register(self.fd, select.POLLIN | select.POLLPRI | select.POLLERR | select.POLLHUP | select.POLLNVAL)
> +        except:
> +            self.close()
> +            raise
>
>      def kill(self, sig):
>          """Send unix signal "sig" to the child process.
> --
> 2.7.0
>