[Interest] TSAN findings in a simple QApplication/QMetaObject::invokeMethod example

[Thiago Macieira]

On Saturday, 5 November 2022 04:41:55 PDT Dennis Luehring wrote:
> but that even raised my TSAN warning amount :(
> 
> 
> Warnings from my example: https://pastebin.com/XnN6nzUT

But they're of higher quality now, with much richer backtraces.

The first one in that link I'm not sure about. The second one is interesting 
because it's claiming that free is racing with malloc. That's... surprising. I 
don't know if TSan is simply broken in your case or if it is missing a mutex 
somewhere that was supposed to show that there's no data race between the 
malloc() caller and the free() caller.

You must have compiled in release mode, because there seem to be some frames 
missing here:

    #1 g_malloc <null> (libglib-2.0.so.0+0x5e718)
    #2 QGtk3ThemePlugin::create(QString const&, QList<QString> const&) /home/
linux/dev/3rdparty-linux-gcc/qt6_dev/qt6/qtbase/src/plugins/platformthemes/
gtk3/main.cpp:22 (libqgtk3.so+0xb55a)

gtk3/main.cpp:22 is a simple new QGtk3Theme line, so the constructor must have 
been inlined. I was going to say that something tail-called into g_malloc... 
but that doesn't make sense, because one usually uses the result of g_malloc, 
and a tail call implies it is leaking. And we know it isn't leaking, because 
the pointer was (likely) used.

Anyway, libglib-2.0.so remains a black box for us. 

> also using
> 
> QT_NO_GLIB=1
> 
> does not reduce the warning amount

It won't help if the Gtk3 theme plugin is in use. Since it's just a plugin and 
you've built your own Qt, the simplest is to just delete the file from 
plugins/.

Skipping all those glib warnings, we get to a race that seems to be strictly 
in Qt. It was even helpful enough to print:

  Location is global 'QCoreApplication::self' of size 8 at 0x7f622f44e2c0 
(libQt6Core.so.6+0x000000b792c0)

The code it is noting is expected: the QCoreApplication destructor resets that 
pointer; other threads are expected to read it. However, it's not an atomic. 
Moreover, the last use from an aux thread came from:

 bool selfRequired = QCoreApplicationPrivate::threadRequiresCoreApplication();
 if (!self && selfRequired)

This is some code I wrote a while ago *for* QtDBus and selfRequired should 
have been false for this thread -- QDBusConnectionManager is in the stack 
trace and QDBusConnectionManager is a QDaemonThread. So why was the boolean 
true? One thing the backtrace doesn't tell us is the timing: it's possible 
that the event being handled here is *exactly* the event that will set that 
boolean to false.

As I said, I added QDaemonThread *for* QtDBus and it looks like I didn't fix 
everything. I also distinctly remember writing this changelog that appeared in 
5.6.0:

 - In Qt 6, QCoreApplication::notify() will not be called for events being
   delivered to objects outside the main thread. The reason for that is
   that the main application object may begin destruction while those
   threads are still delivering events, which is undefined behavior.
   Applications that currently override notify() and use that function
   outside the main thread are advised to find other solutions in the mean
   time.

But I don't remember changing the mechanism in Qt 6. Looks like I failed to 
live up to the promise/threat. And this is exactly the race that your code is 
showing.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Cloud Software Architect - Intel DCAI Cloud Engineering