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

[Stephen Warren]

On 02/14/2016 06:19 PM, Simon Glass wrote:
> 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'.

In the non-error case, s is returned later in the function and continues
to be used by the caller. So, we don't want to tear it down except when
an exception occurred.