avoid unbound local ref on exc before first sample

[maedoc]

Fixes #1712; if sampling is interrupted before first sample is generated, strace is unbound.

This fix assigns strace a sentinel value (None) before the loop. In case of KeyboardInterrupt, strace is closed if required and the interrupt is reraised, avoiding a return MultiTrace([None]).

[ColCarroll]

#1709 adds the pylint ignore line -- since you're fixing this, can you merge and get rid of the pylint comment?

Could you also catch the corner case at the end where sampling is an empty iterator, so strace is still None? In that case, I think returning MultiTrace([]) will provide the most informative error message!

[maedoc]

This assumes sampling never returns None as a reasonable value. If it does, an object() can provide a unique object, e.g.

default_value = object()
strace = default_value
for strace in sampling:
    pass
if strace is default_value:
    pass

[ferrine]

Why so difficult? Add context manager to BaseTrace class

def __enter__(self):
    return self

def __exit__(self, *args): # cannot recall exact signature
    self.close()

UPD: I see some refactoring will be needed though.

[maedoc]

@ferrine that doesn't address the original issue that strace may be a referenced but unbound local variable if (1) the loop is KeyboardInterrupted or (2) sampling is an empty iterator.

[ferrine]

@maedoc That is the usual way for handling cases when we need to do some ending action even when Exception occurred. Here it is closing the backend.

[maedoc]

If adding context manager behavior makes sense for an strace object, then that refactor would be best handled in a separate PR by a core dev. This PR aims to address only #1712.

[ColCarroll]

I think this is good -- thanks a bunch!

@ferrine feel free to add a context manager if you'd like, but it does look tricky to me!