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

[Dawit A]

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.

> 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
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. 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:

static QBasicAtomicPointer<KBookmarkManagerList> _k_static_s_pSelf = { (0) } ;
static bool _k_static_s_pSelf_destroyed;
static struct {
    inline bool isDestroyed() const { return _k_static_s_pSelf_destroyed; }
    inline bool exists() const { return _k_static_s_pSelf != 0; }
    inline operator KBookmarkManagerList *() { return operator->(); }
    inline KBookmarkManagerList *operator->() {
          if (!_k_static_s_pSelf) {
              if (isDestroyed()) {
                  qFatal("Fatal Error: Accessed global static '%s
*%s()' after destruction. " "Defined at %s:%d",
"KBookmarkManagerList", "s_pSelf", "anonymous", 0);
              }
              KBookmarkManagerList *x = new KBookmarkManagerList () ;
              if (!_k_static_s_pSelf.testAndSetOrdered(0, x) &&
_k_static_s_pSelf != x ) {
                  delete x;
              } else {
                  static KCleanUpGlobalStatic cleanUpObject = { destroy };
              }
          }
          return _k_static_s_pSelf;
    }

    inline KBookmarkManagerList &operator*() { return *operator->(); }

  static void destroy() {
        _k_static_s_pSelf_destroyed = true;
        KBookmarkManagerList *x = _k_static_s_pSelf;
        _k_static_s_pSelf = 0;
        delete x;
  }
} s_pSelf;