Opened 13 years ago
Closed 13 years ago
#10766 closed Patch - Bug Fix (Fixed)
Music import fails due to open files not closed
Reported by: | 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)
Change History (7)
comment:1 Changed 13 years ago by
Milestone: | unknown → 0.25.1 |
---|---|
Owner: | set to stuartm |
Status: | new → accepted |
comment:2 Changed 13 years ago by
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 13 years ago by
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?
comment:4 Changed 13 years ago by
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 13 years ago by
Resolution: | → Fixed |
---|---|
Status: | accepted → closed |
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)
lsof | grep /MythTV/music