Opened 7 years ago

Closed 7 years ago

#10766 closed Patch - Bug Fix (Fixed)

Music import fails due to open files not closed

Reported by: mayfields@… Owned by: stuartm
Priority: major Milestone: 0.25.1
Component: Plugin - MythMusic Version: 0.25-fixes
Severity: medium Keywords:
Cc: Ticket locked: no

Description

As per 10721, which was specific to FLAC files, the same issue exists with OGG and WAV files. The same fix should be applied to MetaIOOggVorbis::Read and MetaIOWavPack::Read. I have not tested this to validate as all my music is in FLAC format. May fix ticket 10757.

Attachments (2)

lsof.txt (234.2 KB) - added by Steven Adeff <adeffs.mythtv@…> 7 years ago.
lsof | grep /MythTV/music
patch.fixavfdecoder (744 bytes) - added by mayfields@… 7 years ago.
Fix to avfdecoder.cpp

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by stuartm

Milestone: unknown0.25.1
Owner: set to stuartm
Status: newaccepted

Changed 7 years ago by Steven Adeff <adeffs.mythtv@…>

Attachment: lsof.txt added

lsof | grep /MythTV/music

comment:2 Changed 7 years ago by Steven Adeff <adeffs.mythtv@…>

I attached an lsof output during a fresh music scan, along with this log: http://tincanfury.dyndns.org/users/tincanfury/flac/mythfrontend.20120529131157.10394.log.gz

a lot of files are left open, even though it looks as though they are being closed?

comment:3 Changed 7 years ago by mayfields@…

I suspect this ticket and 10757 are effectively the same, with this ticket being only a partial fix of open files not being closed. 10757 seems to stem from MetaIOID3 not closing files correctly, where this ticket notes files not being closed but only noting MetaIOOggVorbis and MetaIOWavPack. The logs from the previous two comments relate to MetaIOID3 but the end result is the same ie files not being closed and the scan ultimately failing.

The logs are revealing. The mp3 files are being handled without issue, but the FLAC files aren't being closed. Then, once the number of open files builds up the mp3 files are also having issues, but probably only because there are so many flac files still open.

I suspect there is some strange behaviour going on where the file that is currently open is being treated as though it is an mp3 file, because the first file opened was an mp3 file. Is m_file being implicitly cast by the first file opened (in this case to TagLib::MPEG::File)

        case kMPEG :
            m_file = new TagLib::MPEG::File(fname.constData());
            break;
        case kFLAC :
            m_file = new TagLib::FLAC::File(fname.constData());
            break;

hence when m_file is deleted the call is being made to TagLib::MPEG::File rather than TagLib::FLAC::File?

When reading the tags, there is a static cast:

        case kMPEG :
            tag = (static_cast<TagLib::MPEG::File*>(m_file))->ID3v2Tag(create);
            break;
        case kFLAC :
            tag = (static_cast<TagLib::FLAC::File*>(m_file))->ID3v2Tag(create);
            break;

Does this also need to be done for other references to m_file, including when deleting?

Changed 7 years ago by mayfields@…

Attachment: patch.fixavfdecoder added

Fix to avfdecoder.cpp

comment:4 Changed 7 years ago by mayfields@…

OK, not what I thought.

avfdecoder.cpp creates a new MetaIOID3 object, opens a file using it, but doesn't always return it. If it isn't returned, it should be deleted otherwise the file is kept open. Patch for avfdecoder.cpp attached.

There is still one residual issue with the file scanner after the fixes for wav, ogg, and flac MetaIO classes, as well as avfdecoder.cpp.

The file scanner tries to get album art from the tags, and this opens a file. This file is closed when the next file is scanned due to the way MetaIOID3 is implemented, but after the scan the last file stays held open. Perhaps a new ticket is in order for that as it doesn't prevent the scan completing like the other issues do (with large libraries).

comment:5 Changed 7 years ago by stuartm

Resolution: Fixed
Status: acceptedclosed

In 2144ef18beb7024b0e266e3dd5d0ed056ca0cd3a/mythtv:

Fix leak of MetaIOID3() object if we're scanning Flac files without ID3 tags. Fixes #10766 and #10757

In 036d9970b21e71bd1a243ecdc234ca35445587ae/mythtv:

Fix leak of MetaIOID3() object if we're scanning Flac files without ID3 tags. Fixes #10766 and #10757 (cherry picked from commit 2144ef18beb7024b0e266e3dd5d0ed056ca0cd3a)

Note: See TracTickets for help on using tickets.