Skip to content
Commit 436e38ad authored by Harald Sitter's avatar Harald Sitter 🐧
Browse files

kdirwatch: always unref d, and unset d from inside d

previously we could fall into a trap when the current thread for
whatever reason has no local data (e.g. the dtor is invoked from the
wrong thread). when that happened we would leave our d untouched and
never unref, which then causes particularly hard to track down problems
because we can crash at a much later point in time when the d tries to
send an event to the since-deleted KDirWatch instance.

instead let's revisit this a bit...

the reason the `dwp_self.hasLocalData()` check was added is because of
destruction order between QThreadStorage (the d) and QGlobalStatic (the
::self())

specifically because of this caveat from the QThreadStorage
documentation:

> QThreadStorage can be used to store data for the main() thread.
QThreadStorage deletes all data set for the main() thread when
QApplication is destroyed, regardless of whether or not the main()
thread has actually finished.

versus QGlobalStatic:

> If the object is created, it will be destroyed at exit-time, similar
to the C atexit function.

This effectively means that QThreadStorage (at least on the main()
thread) will destroy BEFORE QGlobalStatic.

To bypass this problem the dwp_self check was added to conditionally
skip unrefing when the QThreadStorage had already cleaned up.

The trouble is that this then hides the mentioned scenario where we have
a d but no backing data because of poor thread management.

Instead improve our management of the pimpl: In ~KDirWatchPrivate we now
unset the d pointer of all still monitoring KDirWatch. This allows us to
unconditionally check for d in the ~KDirWatch so it always
unrefs/removes itself from KDirWatchPrivate. In the shutdown scenario
cleanup runs the other way around... KDirWatchPrivate cleans up and
unsets itself as d pointer, thereby turning ~KDirWatch effectively no-op
so when it runs afterwards it won't access any Private memory anymore.

CCBUG: 472862
parent b7d9fac3
Pipeline #510997 passed with stage
in 2 minutes and 58 seconds
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment