ffdec: Avoid AttributeError during destruction

[ssssam]

This fixes a possible exception that looks like this:

Traceback (most recent call last):
  File "/home/sam/src/audioread/audioread/ffdec.py", line 305, in __del__
    self.close()
  File "/home/sam/src/audioread/audioread/ffdec.py", line 290, in close
    self.stderr_reader.join()
AttributeError: 'FFmpegAudioFile' object has no attribute 'stderr_reader'

The close() method is called from this object's destructor, so it has to be handle being called with the object in any state.

[sampsyo]

Seems reasonable! I'm not quite sure I see, however—I read over the __init__ method, and I can't see a point where the constructor would return (without throwing an exception) without assigning to self.stdout_reader and self.stderr_reader.

Do you know what you did to get the object into a state where this exception happened?

[ssssam]

It's a good question, it's not entirely obvious to me how this happened either.

I hit the problem while doing a speed test related to #83, in which I called audio_open() repeatedly on different files without reading any data. I can't reproduce it now at all, even on the original version of my branch for #83.

However, it's notable that the exception was triggered from the __del__() method. A bit of research shows that __del__() is called when exceptions are thrown during __init__(). So perhaps what really happened is that I hit an exception in the constructor at some point, but that triggered a second exception in __del__() which hid the real problem.

With that in mind, perhaps a better solution is to remove the __del__() method altogether? It seems to only exist as a safety rail for code that fails to correctly call the .close() method, in any case.

[sampsyo]

Got it! That would certainly explain it (that __del__ is still called even when there's an exception in __init__).

I think this implies that we do definitely need to do something, and that the appropriate thing to do is what we already have for proc:

if hasattr(self, 'proc'):

I know it seems crazy, but doing the check like this will also protect us against the very surprising scenario when an exception occurs before we can assign None to those fields.

The reason we have a destructor at all is, indeed, as a way to free up resources when they're not closed. I like the symmetry to Python's built-in file objects, as returned by open(): it's not possible to leak filehandles by forgetting a close() call.

[ssssam]

OK! Here's a patch that fixes the issue using hasattr() to avoid errors when the object wasn't fully initialized.

[sampsyo]

Awesome; thank you!! :sparkles: