[mythtv-commits] Ticket #10067: Potential memory leak in EITHelper

[MythTV]

#10067: Potential memory leak in EITHelper
----------------------------------------+-------------------------
     Reporter:  mythtv@…                |      Owner:  stuarta
         Type:  Patch - Bug Fix         |     Status:  new
     Priority:  minor                   |  Milestone:  unknown
    Component:  MythTV - EIT            |    Version:  Unspecified
     Severity:  medium                  |   Keywords:
Ticket locked:  0                       |
----------------------------------------+-------------------------
 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.
 }}}

-- 
Ticket URL: <http://code.mythtv.org/trac/ticket/10067>
MythTV <http://code.mythtv.org/trac>
MythTV Media Center