Skip to content

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

Harald Sitter requested to merge work/sitter/kdirwatchdtor into master

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

Merge request reports