Modify

Opened 6 years ago

Closed 2 years ago

#10067 closed Patch - Bug Fix (Fixed)

Potential memory leak in EITHelper

Reported by: mythtv@… Owned by: stuarta
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. }}}

Attachments (0)

Change History (7)

comment:1 Changed 6 years ago by stuartm

  • Milestone changed from unknown to 0.25
  • Version changed from Unspecified to Master Head

comment:2 Changed 6 years ago by Github

Refs #10067. Fix event dequeing logic issue

Branch: master Changeset: d6f038c56b52a8fda05f13c6be897852cb1b5953

comment:3 Changed 6 years ago by stuarta

  • Milestone changed from 0.25 to 0.26
  • Status changed from new to 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 5 years ago by kenni

  • Milestone changed from 0.26 to 0.26.1

comment:5 Changed 5 years ago by paulh

  • Milestone changed from 0.26.1 to 0.28

comment:6 Changed 4 years ago by Ken <kenkyee@…>

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 2 years ago by dekarl

  • Resolution set to Fixed
  • Status changed from assigned to 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.

Add Comment

Modify Ticket

Action
as closed The owner will remain stuarta.
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.