MP3Decoder: When assigning the "file", close the old one

[jepler]

JP ran into problems with playing multiple MP3 files within the same program. One stumbling point was around the MP3Decoder.file property.

Before this, it was the responsibility of the user to close the old file, which led to inconvenient and non-obvious code such as

    old_file = decoder.file
    decoder.file = open("example.mp3", "rb")
    old_file.close()

Compatibility with this idiom is preserved, as it's permitted to close() a file as many times as desired; the second and subsequent close()s do nothing.

[tannewt]

What happens if you try and use the file afterwards? Seems like there is a weird ownership issue here.

[jepler]

Before this change, if you keep old_file around, it was an open file that you could perform operations on. After it, if you keep old_file around, it is a closed file and most operations raise an exception.

My assumption was that it was important to reach the file's close() / __exit__ or some resource would be leaked (like file descriptor numbers on linux would be, in the absence of reference counting in cpython3). However, this may not be true. The identity of the opened file cycles among a small number of addresses and overall free memory does not decrease in this example:

>>> while True: (open("/boot_out.txt", "rb"), gc.collect(), gc.mem_free())
(<io.FileIO 200168a0>, None, 143616)
(<io.FileIO 200166a0>, None, 143616)…

Given that, this change may not be important. It's something I was carrying locally during the coding of JEplayer, which frequently struggled with running out of memory. When I shipped it, this code had not been PR'd, but the "manual close" every time I wanted to change MP3 streams remained.

[tannewt]

I'm worried it's not clear that the file is closed when set. It also seems counter-intuitive to close a file that is still in use.

I think the best thing is to omit this. Do you agree?

[jepler]

JP verified that his code is working without the manual close so I guess it's okay.