Review Request: Fix a VLC crash by delaying object deletion to avoid invalid access by QtDBus...

[Dawit A]

On Sat, Feb 5, 2011 at 2:46 PM, Thiago Macieira <thiago at kde.org> wrote:
> On Saturday, 5 de February de 2011 14:15:03 Dawit A wrote:
>> On Sat, Feb 5, 2011 at 12:57 PM, Thiago Macieira <thiago at kde.org> wrote:
>> > On Saturday, 5 de February de 2011 17:39:35 Dawit Alemayehu wrote:
>> >> was that in both cases the backtrace shows QtDBus attempting to invoke
>> >> or access some method or slot in the object that was just destroyed.
>> >> At this
>> >
>> > That's supposed to be impossible. QtDBus connects to the object's
>> > destroyed(QObject*) signal, and removes the object from the object tree
>> > when it's destroyed.
>> >
>> > It's also thread-safe, as all manipulations of the object tree are done
>> > under a write lock.
>>
>> The cause could be the fact that KIO itself is not really thread-safe.
>> I do not really know that for sure, but from what you described above,
>> it would seem to me to be some sort of race condition which is a
>> complete possibility in KIO (not thread safe). All I can say with 100%
>> certainty is that deferring the deletion of the objects that register
>> with QtDBus most definitely solves this bug, at least for me.
>
> QtDBus only calls any methods in the QObject in the QObject's proper thread.
> There's no chance of it being the wrong thread here either.
>
>> > The only thing I can think of is that something caused the signal to
>> > disconnect.
>>
>> Wouldn't the object being deleted cause the disconnect ? Dunno at what
>
> Yes, but only *after* the slots connected to destroyed() were invoked. I mean
> that something disconnected the QDBusConnectionPrivate slot *before* deletion.
> The object was then deleted and a method invoked in it.
>
>> point that would happen, but as you can see from the backtraces posted
>> in BR# 234484, the QtDBus calls come after the object that registered
>> with it is destructed.
>
> Which shouldn't happen, by construction.
>
>> Oh and I forgot to mention a very important
>> point. The deletion of both KBookmarkmanager and KIO::Scheduler, the
>> obejcts that register with QtDBus, originates from the destruction
>> KIO::SchedulerPrivate and KBookmarkManagerList respectively. Both
>> these objects are global static objects created through
>> K_GLOBAL_STATIC. Here is what KBookmarkManagerList's global static
>> marco declaration expands to:
> [snip]
>
> No need, I designed K_GLOBAL_STATIC with Matthias Kretz back in the day.
>
> Anyway, I can reproduce the crash on my office computer, even when running the
> multithreaded waitpid solution. I will try to valgrind the issue when I'm in
> the office on Tuesday.

Thiago, did you ever get around to doing this or should I commit my workaround ?