[mythtv] Re: Ticket #341: Music database update does not reread metadata

[Daniel Kristjansson]

On Thu, 2005-10-27 at 00:47 -0700, Eskil Heyn Olsen wrote:

> I was under the impression that every object in the QObject hierachy
> should declare the Q_OBJECT macro. However, most of the MythWidget
> classes don't. What does this do ?
If you don't declare a Q_OBJECT macro you can't declare signals and slots,
simple as that. In MythTV we try to only use Q_OBJECT in header declared
classes, this allows the build system to pick up the signals and slots
without hacks to force qmake to pick these from .cpp files.

We usually only add a Q_OBJECT macro unless it is actually needed,
but this is mostly because it is one less line of code to write/read.
It's not that we are worried about the <2KB code bloat.

> I understand how deleting an object can cause a timer that triggers
> later to sigsegv, if it refers to the deleted object. But why would
> deleteLater cause the subsequent havoc ? What I saw in the flurry of
> patches was;
deleteLater() only prevents one kind of signal/slot deletion problem.
It prevents the Qt event thread from sending a signal to the object
after the object is actually deleted. But it does that by just deleting
all the objects in the deleteLater queue after firing off all the events
for the time slice.

The reason deleteLater() might make things worse as far as crashing
is that it doesn't disconnect signals/slots right away by default.

So you have a QTimer and connect it to MythBusyDialog, then you call
deleteLater on the QTimer.. now if you actually use delete on
MythBusyDialog before the Qt event thread deletes the QTimer, then
the timer might expire and fire an event to the MythBusyDialog.

There are three fixes for this:
* One fix is to call disconnect() on any QObject derived class
  before calling deleteLater() on it. This prevents the it from
  sending any events after you consider it deleted.
* Another fix used in some of the MythTV QObject derived classes
  is implement your own deleteLater() that calls disconnect() on
  itself and then calls the parent's deleteLater() on itself.
* The correct thing to do is always call deleteLater() on all
  QObject derived classes.

In your case you, calling deleteLater() on the timer in the 
MythBusyDialog destructor without disconnecting it first
guarantees that things are unsafe in case of timer event.
(These are fired by the Qt event thread.)

I really should have caught this. :/
Anyway, I just committed a Q&D fix.

BTW There are other problems with QObjects:
* Most notably you can only have one in your inheritance lineage,
  so no multiple inheritance using two or more QObject derived classes.
* Also, you can not virtually inherit a QObject derived class. This is
  what I tried after some compile problems doing multiple inheritance.
  The compiler allowed this, but we started getting strange crashes...
  Turns out signals were being sent to the wrong slots...

-- Daniel