Opened 6 years ago

Closed 6 years ago

#12052 closed Bug Report - Crash (Fixed)

MythTV crashes when the filter in video list (tree) view is called after a video was deleted

Reported by: angela.schmid@… Owned by: Richard Hulme <peper03@…>
Priority: blocker Milestone: 0.27.1
Component: MythTV - Video Library Version: Master Head
Severity: high Keywords: crash filter tree view
Cc: Ticket locked: no

Description

Since 0.25 when deleting a video (in whatever view) and the filter is called in list (tree) view MythTV crashes.

There are several scenarios to crash MythTV, the easiest way to reproduce:

Create a video file, in list (tree) view, scan, delete the file (if not found set the filter accordingly), call the filter dialog, press OK.

Attachments (4)

gdb.txt (59.9 KB) - added by angela.schmid@… 6 years ago.
gdb.2.txt (63.2 KB) - added by angela.schmid@… 6 years ago.
0001-Only-force-a-refresh-of-the-metadata-rather-than-inv.patch (954 bytes) - added by peper03 6 years ago.
gdb.3.txt (63.2 KB) - added by angela.schmid@… 6 years ago.

Download all attachments as: .zip

Change History (19)

Changed 6 years ago by angela.schmid@…

Attachment: gdb.txt added

comment:1 Changed 6 years ago by stuartm

Milestone: unknown0.27.1
Priority: minorblocker
Severity: mediumhigh

comment:2 Changed 6 years ago by Richard Hulme <peper03@…>

Owner: set to Richard Hulme <peper03@…>
Resolution: fixed
Status: newclosed

In ffd38d1bba28aea710ad095c4e0844fe63381b38/mythtv:

Invalidate the cache after deleting a video file to ensure the free'd entry is removed.

Fixes #12052

comment:3 Changed 6 years ago by Richard Hulme <peper03@…>

In de615b3360219838e446ae75328730c2ff49d64f/mythtv:

Invalidate the cache after deleting a video file to ensure the free'd entry is removed.

Fixes #12052
(cherry picked from commit ffd38d1bba28aea710ad095c4e0844fe63381b38)

comment:4 Changed 6 years ago by angela.schmid@…

Tested on fixes/0.27, patch doesn't fix problem. Immediately after deleting the video, MythTV crashes.

Changed 6 years ago by angela.schmid@…

Attachment: gdb.2.txt added

Changed 6 years ago by peper03

comment:5 Changed 6 years ago by peper03

I'm assuming you have 'Browse Filesystem' turned on as this was the only way I could observe a crash with the original patch.

Please try the new patch. That works for me whether 'Browse Filesystem' is enabled or disabled.

comment:6 Changed 6 years ago by angela.schmid@…

I don't have Browse Filesystem enabled. The second patch does not work.

https://code.mythtv.org/trac/attachment/ticket/12052/0001-Only-force-a-refresh-of-the-metadata-rather-than-inv.patch does not differ from https://code.mythtv.org/trac/changeset/ffd38d1bba28aea710ad095c4e0844fe63381b38/mythtv as m_metadata_list_type = VideoListImp::ltNone is within InvalidateCache?().

            if (ret)
            {
                ret = m_metadata.purgeByID(video_id);
                InvalidateCache();
                // Force refresh
                m_metadata_list_type = VideoListImp::ltNone;
            }
        }

        return ret;
    }

    void InvalidateCache() {
        // Set the type to none to avoid refreshList thinking it doesn't
        // need to.
        m_metadata_list_type = VideoListImp::ltNone;

        metadata_list ml;
        VideoMetadataListManager::loadAllFromDatabase(ml);
        m_metadata.setList(ml);
    }

Changed 6 years ago by angela.schmid@…

Attachment: gdb.3.txt added

comment:7 Changed 6 years ago by peper03

Resolution: fixed
Status: closednew

Are you doing anything different to the method described in the original description (Scan, set a filter, delete, try to set a filter)?

In case it's related to the theme, which theme are you using?

comment:8 Changed 6 years ago by angela.schmid@…

I tested on fixes/0.27 and master, with Arclight and MyhtCenter?-wide. With the patch, within gallery view, Scan, set a filter (getting more videos), delete, crash. The list is not updated on the screen.

comment:9 Changed 6 years ago by peper03

Are you browsing by folder or by one of the other options (e.g. genre, category etc.)? Is the file you're deleting in a sub-folder or at the top level?

I cannot reproduce the crash here so there must be a specific set of conditions in your setup that I don't currently have.

The second patch is not the same as the first patch. InvalidateCache?() does more than only set m_metadata_list_type.

comment:10 Changed 6 years ago by angela.schmid@…

The second patch is not the same as the first patch. InvalidateCache?() does more than only set m_metadata_list_type.

But "m_metadata_list_type = VideoListImp::ltNone" is called twice, within Invalidate Cache and after it returns in Delete(). For me this is unnecessary, do I oversee something here?

I am in gallery view, browse by has no influence, as I use the flatten directory option. The problem also exists in list view, where I have browse by folder set, independent if the file is at the top level or in a sub-folder.

As it crashes with the patch originating from videodlg.cpp (gdb2.txt), does videodlg keep a reference at videolist metadata, but doesn't know about changes?

comment:11 Changed 6 years ago by peper03

Sorry, I missed that. The second patch hasn't been applied cleanly. The second patch *removes* the call to InvalidateCache? and replaces it with the m_metadata_list_type line.

Take out the call to InvalidateCache? and try again please.

comment:12 Changed 6 years ago by angela.schmid@…

The mistake is with me, I didn't properly read the patch, i missed the red, to be deleted lines. I read the patch and focused only on what to add.

I removed the call to InvalidateCache? and it didn't crash. Thanks.

As there were several scenarios producing the crash I will keep an eye on it.

comment:13 Changed 6 years ago by Richard Hulme <peper03@…>

In d7bee2a3cca4a76feade96a29094a3eb3affb23b/mythtv:

Only force a refresh of the metadata rather than invalidating the cache as that causes a crash if file system browsing is enabled.

Refs #12052

comment:14 Changed 6 years ago by Richard Hulme <peper03@…>

In d48bbd891921d33f5fb34ccc46044ad8987183fb/mythtv:

Only force a refresh of the metadata rather than invalidating the cache as that causes a crash if file system browsing is enabled.

Refs #12052
(cherry picked from commit d7bee2a3cca4a76feade96a29094a3eb3affb23b)

comment:15 Changed 6 years ago by peper03

Resolution: Fixed
Status: newclosed
Note: See TracTickets for help on using tickets.