[Bug 279135] Phonon-gstreamer assertion "Cannot send events to objects owned by a different thread." [@ Phonon::Gstreamer::X11Renderer::movieSizeChanged]

[Harald Sitter]

On Tue, Aug 2, 2011 at 6:32 PM, Trever Fischer
<tdfischer at fedoraproject.org> wrote:
> On Tuesday, August 02, 2011 11:49:41 AM Harald Sitter wrote:
>> On Tue, Aug 2, 2011 at 4:50 PM, Trever Fischer
>>
>> <tdfischer at fedoraproject.org> wrote:
>> > --- Comment #1 from Trever Fischer <tdfischer fedoraproject org>
>> >  2011-08-02 14:50:17 --- Git commit
>> > e08b43c99ff4a1e2705f697249804e19ef9f846f by Trever Fischer. Committed on
>> > 02/08/2011 at 16:45.
>> > Pushed by tdfischer into branch 'master'.
>> >
>> > Sometimes we get the cb_capsChanged handler in a different thread.
>> >
>> > BUG:279135
>> >
>> > M  +3    -1    gstreamer/videowidget.h
>> > M  +3    -2    gstreamer/videowidget.cpp
>> >
>> > http://commits.kde.org/phonon-gstreamer/e08b43c99ff4a1e2705f697249804e19e
>> > f9f846f
>>
>> I think we should make it a policy to always use invokeMethod in
>> callbacks, or if there is a good reason not to do so, that it needs
>> documenting why invokeMethod would not make sense. Perhaps even a
>> queued invocation by policy?
> For this instance, it seems that the problem stemmed from a QEvent being
> constructed in a thread that the QObject didn't live in. This is generally a
> rare occurance and only happens when you're on the other side of a queue. In
> this case, it was a QResizeEvent being created because the code was calling
> setGeometry from another thread.

That is exactly the problem. By calling functions from a different
thread directly you get weird results at different ends.

>> In like every case I have seen a callback not use invokeMethod it
>> either dragged a different thread context through all sorts of
>> not-thread-safe components and then resulted in an obscure segfault,
>> or it did not segfault but leak memory somewhere along the way, or it
>> did cause premature signal emission.
> The base QObject class (especially signals/slots) is reentrant, so I'm not
> sure what unsafe components a thread might be dragged through.

So it is, that does not mean that the functions executed as result of
a signal emission (directly from a foreign thread or indirectly via a
call chain from that thread ending in an emit) are. Worst case:
someone does a direct connection -> kaboom. Also I have seen cases
(with pulseaudio callbacks) in which QThread reported the same current
thread as a target object while in fact the thread was different
(particularly with glib, and I reckon even more so when the QEventLoop
is not also using glib). This consequently renders the autoconnection
feature of signals/slots useless and you end up with a foreign thread
in the slot.

>> Opinions?
> Making it a policy to use invokeMethod to cleanly sidestep the threading issue
> is a possibility, but seems like more overhead to me. The new Pipeline class
> did away with a lot of that as all of its operations are reentrant. Since it
> is its own object, other objects connect to it and Qt automatically makes them
> queued since the events are posted from a different thread.

...if detected. Making it a policy would be more about raising
awareness that one is working on a different thread and that it is
necessary to make the code suited for this.

For example, if you insert in the unlock() method of pvlc's audiodataoutput
    Q_ASSERT(QThread::currentThread() == cw->thread());
and then run dragon of 4.7 with QT_NO_GLIB=1 .... no kaboom :?

Now this might very well be limited to unix when not using glib, so
perhaps all is good on other systems, but without testing who'd know.

Direct function calls are affected of thread mismatch eitherway, so in
terms of making our software more robust either using invokeMethod OR
making sure that the call chain is thread safe seems like a worthwhile
investment.

> If I remember correctly, the original MediaObject had invokeMethod wrappers
> everywhere because sometimes the underyling object got deleted in the middle
> of a signal handler by libphonon. Upon destruction of the Pipeline class, it
> unregisters all the signal handlers from the GstBus. Internally, this uses the
> same mutex as the one used when a signal is being handled, preventing any race
> conditions until the handler finally returns.

Tjis is not particularly a phonon gstreamer, nor a phonon issue. But
generally the case when dealing with callbacks. For instance a kmix
memleak I fixed recently was also due to a call chain from a pulse
thread without QObject ever noticing.
Since we deal a lot with callbacks in multimedia I think we should all
become more aware of the implications.

regards,
Harald
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia