Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-21715

Possible crash in ~QPointer() when it refers to QObject from another thread

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • Not Evaluated
    • 5.0.0
    • 4.7.4
    • Core: Object Model
    • None

    Description

      There is no explicit notice in documentation, is it safe to store in QPointer reference to QObject that lives in another thread. Since QMetaObject uses mutex internally to access GuardHash, I'm assuming that it should be allowed.

      Here is what I'm doing:

      Thread 1:

      {
      QObject *obj = new QObject();
      ...
      delete obj;
      }
      

      Thread 2:

      {
      	QPointer<QObject> ptr(obj); // Get reference to obj from "thread 1"
      	...
      }
      

      In "Thread 2" ~QPointer calls QMetaObject::removeGuard() which firstly checks that pointer is not NULL and then locks shared mutex.

      At the same time "Thread 1" in ~QObject will call QObjectPrivate::clearGuards that locks same mutex and then clears all weak references to object.

      It may happens that "Thread 1" will clear such reference whilel thread will be in destructor of QPointer after checking for NULL but before locking of mutex.

      In such case Thread 1 will dereference NULL pointer at the end of removeGuard().

      It pretty looks like that removeGuard() tries to avoid overhead of locking if QPointer is already NULL. I think that we should fix removeGuard() to check ptr for NULL twice: before and after obtaining of lock. This still avoids extra locking when ptr is NULL and fixes described crash.

      I'm attaching proposed patch

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            earthdomain Earth Domain (Inactive)
            dion Dmitry Nezhevenko
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes