Opened 14 years ago
Closed 10 years ago
#10067 closed Patch - Bug Fix (Fixed)
Potential memory leak in EITHelper
Reported by: | Owned by: | Stuart Auchterlonie | |
---|---|---|---|
Priority: | minor | Milestone: | 0.28 |
Component: | MythTV - EIT | Version: | Master Head |
Severity: | medium | Keywords: | |
Cc: | Ticket locked: | no |
Description
In EITHelper.cpp - EITHelper keeps a list of incomplete_events, and unmatched_etts. If for some reason when parsing the EIT and ETT tables, it does not manage to match any ETT to an EIT entry, the unmatched item will remain on the list indefinitely.
incomplete_events stores character strings allocated with new. If the EITHelper is destroyed, those will be unreferenced and simply leaked.
If EITHelper is not destroyed, incomplete_events and unmatched_etts can grow without bounds. Aside from the memory consumption, that has another unwanted side effect that leftover unmatched items from earlier calls can be picked up as incorrect matches on later calls. This will result in program descriptions being updated with descriptions from other shows - a behavior I'm seeing, which is the whole reason I started looking at EITHelper.
Evidence of the leak can be seen by turning on verbose logging for EIT and watching for this log message:
VERBOSE(VB_EIT, LOC + QString("Added %1 events -- complete(%2) " "incomplete(%3) unmatched(%4)") .arg(insertCount).arg(db_events.size()) .arg(incomplete_events.size()).arg(unmatched_etts.size()));
In the same method, there is a logic bug when iterating over events. The for loop compares the index with the size of the db_events queue, but at the same time the loops is removing events. Thus the index is incremented and the size is decreased, so the loop will always end with only half of the events processed.
So this:
for (uint i = 0; (i < kChunkSize) && (i < db_events.size()); i++) { DBEventEIT *event = db_events.dequeue();
should become this:
for (uint i = 0; (i < kChunkSize) && (db_events.size() > 0); i++) { DBEventEIT *event = db_events.dequeue();
I don't think an ETT table from one event can legitimately be related to an EIT from a different event, so I believe after each iteration of that loop, incomplete_events and unmatched_etts should be cleared - taking care to free the memory associated with items stored in incomplete_events. }}}
Change History (7)
comment:1 Changed 13 years ago by
Milestone: | unknown → 0.25 |
---|---|
Version: | Unspecified → Master Head |
comment:2 Changed 13 years ago by
comment:3 Changed 13 years ago by
Milestone: | 0.25 → 0.26 |
---|---|
Status: | new → assigned |
The second part of this, to remove incomplete events and unmatched_etts can wait till 0.26 as I have no means of testing this one.
Stuart
comment:4 Changed 13 years ago by
Milestone: | 0.26 → 0.26.1 |
---|
comment:5 Changed 12 years ago by
Milestone: | 0.26.1 → 0.28 |
---|
comment:6 Changed 12 years ago by
It's fairly easy to reproduce w/ a dual HDHomerun ATSC tuner. I see this random EIT description quirk fairly often after leaving the MythTV box on for a week or more...
Not sure it should be pushed off to 0.28...
comment:7 Changed 10 years ago by
Resolution: | → Fixed |
---|---|
Status: | assigned → closed |
Closing this as the second part has been superseded by #11739. Marking as fixed for the first part.
Ken, its been pushed as us DVB-country-devs (who do the most work around EIT) are having a hard time testing stuff that only applies to ATSC-countries.
Refs #10067. Fix event dequeing logic issue