Opened 12 years ago

Closed 12 years ago

#3752 closed defect (fixed)

Segfault scheduling recording from guide grid

Reported by: stuartm Owned by: Isaac Richards
Priority: minor Milestone: 0.21
Component: mythtv Version: head
Severity: high Keywords:
Cc: Ticket locked: no

Description

I've been able to reproduce this quite easily but as scheduling recordings this way isn't unusual I cannot explain why it has only just started segfaulting. Go through the menus to the Guide Grid, press return on a currently running program to bring up the recording options, select record only this showing and then save the schedule, at this point it segfaults.

Attachments (2)

backtrace2.txt (9.1 KB) - added by stuartm 12 years ago.
Backtrace
3752.patch (1.5 KB) - added by Anduin Withers 12 years ago.
Do delete notification in deleteLater rather than ~ScheduledRecording?. Prevents no parent when other related deleteLater calls are made.

Download all attachments as: .zip

Change History (17)

Changed 12 years ago by stuartm

Attachment: backtrace2.txt added

Backtrace

comment:1 in reply to:  description ; Changed 12 years ago by will@…

Replying to stuartm:

I downloaded SVN last night and have noticed the same problem. I can re-create on any type of scheduling in the guide grid. The schedule does make it in to the database as reloading the frontend and going back in to the guide shows the programme as flagged for recording.

Do you require another backtrace from me?

Please let me know if you would like any patches tesing.

comment:2 in reply to:  1 Changed 12 years ago by will@…

Sorry forgot to add, I'm 32bit and I think (from the backtrace) that you are 64?

Running latest Ubuntu with no non-ubuntu-provided libs etc.

comment:3 Changed 12 years ago by stuartm

Could you try revert scheduledrecording.cpp/h to [13886] and seeing if that helps? It worked here which means the problem changeset is [13887].

It works here but I'd like to confirm it before I either revert [13887] or see if gigem has a solution.

comment:4 Changed 12 years ago by gigem

I much prefer you investigate further before reverting [13887]. That change fixes a very real problem. I suspect we mighe be dealing with differences in how Qt handles deleteLater in different versions. FWIW, I'm using Debian libqt3-mt 3:3.3.7-5.

comment:5 Changed 12 years ago by stuartm

I'm using QT 3.3.8

I'd rather find a fix, but if we don't do so quickly then reverting [13887] until then won't be bad idea. IMHO a segfault, especially in a critical area beats a (small) mem leak.

comment:6 Changed 12 years ago by gigem

I think you misunderstand the changes. [13823] is the fix for the memory leak. [13887] is the fix for a segfault uncovered by [13823]. What you're proposing is changing one segfault for another. I don't find that acceptable.

comment:7 Changed 12 years ago by anonymous

Hope this doesn't count as a 'me too' but I wanted to point out that I get the segfault after any type of frontend schedule change. eg. 'programme finder', edit of an "upcoming recording", as well as when I use the guide grid.

Running 13961 on an up-to-date 32-bit Fedora FC6 install.

comment:8 in reply to:  6 Changed 12 years ago by anonymous

Replying to gigem:

I think you misunderstand the changes. [13823] is the fix for the memory leak. [13887] is the fix for a segfault uncovered by [13823]. What you're proposing is changing one segfault for another. I don't find that acceptable.

Yes, sorry, I mis-read the commit message. I should stop posting when tired, it _always_ gets me in trouble.

I'm no less tired now, so I might be wrong and should probably stop writing, but would the second deleteLater cause a segfault? iirc it's safe to call deleteLater more than once.

comment:9 Changed 12 years ago by gigem

It appears [13957] is the change that introduces the latest segfault. That should be just a simple syntactical change to pacify older versions of gcc, but apparently causes a different double free somewhere.

Unless someone who knows the maze of multiple inheritance that is the Myth settings and widget classes better than me can step up, I'm inclined to revert all 3 changes in question. Those are [13957], [13887] and [13823]. That would reintroduce the memory leak, but as you said already, that's better than segfaults.

comment:10 Changed 12 years ago by stuartm

If [13957] is the problem commit then I really don't understand how but reverting it would cause the least problems.

[13823] is perfectly correct and so I'd leave that.

If I better understood the reason for the segfault caused by [13823] then I might understand [13887] a little better. Do you have a backtrace for that one?

comment:11 in reply to:  10 Changed 12 years ago by gigem

Replying to stuartm:

If [13957] is the problem commit then I really don't understand how but reverting it would cause the least problems.

It is needed to avoid a compile error for some people.

[13823] is perfectly correct and so I'd leave that.

If I better understood the reason for the segfault caused by [13823] then I might understand [13887] a little better. Do you have a backtrace for that one?

I don't have one any more, but it is trivial to reproduce. Go to upcoming recordings, then press i to brign up the SR dialog. Press escape to abort without changing anything. Press i again to bring the dialog back up. Poof.

comment:12 Changed 12 years ago by will@…

I've reverted scheduledrecording.cpp/h to 13886 with:

svn up -r 13886 libs/libmythtv/scheduledrecording.*

And I can confirm that I can now schedule recordings without the seg fault.

I hope this helps you to narrow down what's going on.

Changed 12 years ago by Anduin Withers

Attachment: 3752.patch added

Do delete notification in deleteLater rather than ~ScheduledRecording?. Prevents no parent when other related deleteLater calls are made.

comment:13 Changed 12 years ago by Anduin Withers

Please try the attached patch, it should force child delete calls to happen in the proper order.

comment:14 Changed 12 years ago by will@…

The patch stops the seg fault.

Thank you!

comment:15 Changed 12 years ago by gigem

Resolution: fixed
Status: newclosed

Closed with [14021].

Note: See TracTickets for help on using tickets.