QApplication in std::thread

Suthiro

Thank to you all very much for your help and time! Since there are 22 (!) replies I'm sorry in advance if I missed something I need to reply to.

@JKSH

Quick and dirty hack: Comment out delete m_qPlotsControl; -- Does this stop the crash?

I tried to delete, not to delete, to allocate everything (QApplication, QPushButton, QPlotsControl) on heap and on stack in all possible combinations. Crash persists.

Ignore the message for now and see if you can get a stack trace for the crash. I haven't seen "pointers to a deep space" before -- Can you post the exact error messages?

"An exception at address 0x000007FED396B168 raised in lua.exe: 0xC0000005: access rights violation during execution at 0x000007FED396B168." (original in Russian :"Вызвано исключение по адресу 0x000007FED396B168 в lua.exe: 0xC0000005: нарушение прав доступа при исполнении по адресу 0x000007FED396B168.")

Stack trace:

000007fed396b168()
kernel32.dll!000000007702556d()
ntdll.dll!000000007718372d()

Thread: Win64 (working thread). It is neither the thread where QApplication is created, nor the "main" thread of the dll.
Addresses, of course, change from run to run.

Crash will not occur if the button was not interacted. A QTabWidget with custom widgets and buttons and whatever is interactable without problems.
About "pointers to a deep space": I meant dangling pointers, sorry for not being serious & clear.

// IMPORTANT: Make sure that all widgets are heap-allocated, not stack-allocated.
// Call this from your main thread when you're ready to quit.

the message about timers is issued, but no crash occurs. It is really great!

Also, it looks like that only top-level widget must be heap allocated. QPushButton could be a stack-allocated member variable.
ctor contains now exactly one line aside the initializers:

QPushButton QPlotsControl::m_Btn;
QPlotsControl::QPlotsControl(const WorkerThreadCallbackFn& cbFunc, const readyCallbackFn& readyCb,
	QWidget *parent) : m_Callback{ cbFunc }, m_readyCallback { readyCb },
	m_needsClosing{false},
	QMainWindow(parent) { m_Btn.setParent(this); }

Again, no crash if QMetaObject::invokeMethod is used. I think that children are destroyed properly.

Important detail: If we don't interact with the QPushButton, then we don't get the message about timers.

And no crashes!

I believe it is fixable with m_qPlotsControl->deleteLater();... but that needs to be done before QApplication::exec() returns, because QDeferredDeleteEvent won't be processed without an event loop.

It is fixable, thank you!

Since the widget creates a timer in the background, then it's probably receiving background events/signals. delete could cause the event/signal handler to act on a dangling pointer -- that's how it could generate a crash. And that's why deleteLater() can fix the crash. (Alternatively, we could change from a heap-allocated widget to a stack-allocated widget)

Stack allocated widget (actually, even all of them) is not enough. Just re-checked. I do really need to call that QMetaObject::invokeMethod to close the QApplication without crash.

@kshegunov

did you follow the threads?

Yes, I did. The link that you've provided points to the code which is very similiar if not identical to that I use: create QApplication in a std::thread.

Which if you notice passes the argc and argv along from main() to the thread. You should do that too.

There is no argc/argv in a library.

Can you give me a stack trace from that message?

Please, see above.

@Chris-Kawa

I would consider this a Qt bug and expect it to crash all over the place.

@kshegunov

Yes, this is constructed appropriately, however the deallocation should've been tied to the QCoreApplication::aboutToQuit. I doubt anyone is going to bother fixing it though, I'm not sure if it's not going to be discarded with "this is unsupported" even ...

@JKSH

Me too, which is what I wrote somewhere upstairs - this should be fixed at the vendor side.

As I understand, this is a bug in Qt after all. A workaround is to use QMetaObject::invokeMethod described by @JKSH. But are there any side effects?

Thank you all once again!

kshegunov

I am now very confused. How and why QMetaObject::invokeMethod works, what it calls and what it fixes ...?

@Suthiro said in QApplication in std::thread:

Thread: Win64 (working thread). It is neither the thread where QApplication is created, nor the "main" thread of the dll.
Addresses, of course, change from run to run.

You receive a SIGSEGV, which is in a thread that you don't own, it's a system one from the kernel. This means nothing.

There is no argc/argv in a library.

Which is also why QApplication instances should not be created in a library. Please describe what you're doing, and why! I admit we got carried away tracing the technical problem, but it does sound to me you're doing something very wrong ideologically.

JKSH

@kshegunov said in QApplication in std::thread:

I am now very confused. How and why QMetaObject::invokeMethod works, what it calls and what it fixes ...?

Please read my very first post in this thread.

Which is also why QApplication instances should not be created in a library. Please describe what you're doing, and why! I admit we got carried away tracing the technical problem, but it does sound to me you're doing something very wrong ideologically.

It's uncommon for sure, but it's certainly a valid use-case.

Judging from @Suthiro's error message ("An exception at address 0x000007FED396B168 raised in lua.exe), he is making a Qt-based plotting library to be used in a Lua program. Since Lua is a different language, the C++ bits need to be wrapped up in a library. And since Lua controls the main thread, the blocking event loop needs to be shoved into a secondary thread.

JKSH

@Suthiro said in QApplication in std::thread:

the message about timers is issued, but no crash occurs. It is really great!

Wonderful! :-)

Please mark this thread as "Solved".

As I understand, this is a bug in Qt after all. A workaround is to use QMetaObject::invokeMethod described by @JKSH. But are there any side effects?

The only side-effect I can think of is that you need to maintain a few more lines of code.

My code simply calls deleteLater() on every top-level widget in your program (this automatically deletes child widgets too), before quitting the QApplication.

Please help me test a simpler version (Comment out lines #1 and #2 below to trigger automatic quitting via quitOnLastWindowClosed) -- does this still work?:

// IMPORTANT: Make sure that all widgets are heap-allocated, not stack-allocated.
// Call this from your main thread when you're ready to quit.
QMetaObject::invokeMethod(qApp, []
{
    // qApp->setQuitOnLastWindowClosed(false); // #1

    for (auto w : qApp->topLevelWidgets())
        w->deleteLater();

}, Qt::BlockingQueuedConnection);

// QMetaObject::invokeMethod(qApp, &QApplication::quit, Qt::BlockingQueuedConnection); // #2

I tried to delete, not to delete, to allocate everything (QApplication, QPushButton, QPlotsControl) on heap and on stack in all possible combinations. Crash persists.

...

Stack allocated widget (actually, even all of them) is not enough. Just re-checked. I do really need to call that QMetaObject::invokeMethod to close the QApplication without crash.

I see, thank you for testing.

	QMainWindow(parent) { m_Btn.setParent(this); }

Oh, please don't call setParent() on a widget that is not created with new. It can cause a different type of crash because the widget might get deleted twice.

See https://doc.qt.io/qt-5/objecttrees.html#construction-destruction-order-of-qobjects

JKSH

[P.S. This thread has been split -- the other part about stopping timers is now at https://forum.qt.io/topic/126168/global-static-qpixmapcache-in-qt-internals ]

Suthiro

@kshegunov

I am now very confused. How and why QMetaObject::invokeMethod works, what it calls and what it fixes ...?

Please, see what @JKSH wrote in #2 above.

You receive a SIGSEGV, which is in a thread that you don't own, it's a system one from the kernel. This means nothing.

That is exactly why I have not posted it initially.

Which is also why QApplication instances should not be created in a library. Please describe what you're doing, and why! I admit we got carried away tracing the technical problem, but it does sound to me you're doing something very wrong ideologically.

I need a plugin (dll) with non-blocking main. An external third party program utilizes lua scripting, that's why I'm debugging it using "pure" lua.exe. The same crash occurs either using lua.exe and that program. If Qt's message loop is created in the function that being invoked by an external application, the application will wait for return of dllMain() forever and hang. That's why I need a Qt library with QApplication in a separate thread. I also could use QApplication in the main thread, but don't know how to handle the message loop properly in this situation. For the rest part of the library the location of QApplication does not matter, since I'm already using signals/slots & callbacks for communications between threads.

@JKSH

Please mark this thread as "Solved".

Okay, but will this thread stay open for further replies? Asking just in case and for further occasions. I have not tested with third-party program yet (takes too much time to prepare the data in the program), but based on my previous experience it is identical to pure lua.

The only side-effect I can think of is that you need to maintain a few more lines of code.
My code simply calls deleteLater() on every top-level widget in your program (this automatically deletes child widgets too), before quitting the QApplication.

This is really great!

Please help me test a simpler version (Comment out lines #1 and #2 below to trigger automatic quitting via quitOnLastWindowClosed) -- does this still work?:

It does! Thanks again.

Oh, please don't call setParent() on a widget that is not created with new. It can cause a different type of crash because the widget might get deleted twice.

No no, it was a just a test case! Never doing that in real life :)

JKSH

@Suthiro said in QApplication in std::thread:

Please mark this thread as "Solved".

Okay, but will this thread stay open for further replies?

Yes, we can continue to reply after the thread is marked "Solved".

Asking just in case and for further occasions. I have not tested with third-party program yet (takes too much time to prepare the data in the program), but based on my previous experience it is identical to pure lua.

If you encounter further issues, it is best to post in a new thread anyway, since it is probably more complex than the example you posted in this thread.

Please help me test a simpler version (Comment out lines #1 and #2 below to trigger automatic quitting via quitOnLastWindowClosed) -- does this still work?:

It does! Thanks again.

No problem. Now, here's an even better way that doesn't require QMetaObject::invokeMethod() (thanks to @kshegunov). Does it still work if you remove QMetaObject::invokeMethod() and connect deleteLater() to aboutToQuit()?

QApplication a(argc, &argv);
...
m_qPlotsControl = new QPlotsControl(...);
m_qPlotsControl->show();

QObject::connect(&a, &QApplication::aboutToQuit, m_qPlotsControl, &QWidget::deleteLater);

a.exec();

No no, it was a just a test case! Never doing that in real life :)

Good to know :-D

Suthiro

@JKSH

QObject::connect(&a, &QApplication::aboutToQuit, m_qPlotsControl, &QWidget::deleteLater);

Nope, it crashes. I think that aboutToQuit() is executed too late, probably in the same time when "ordinary" destructor would have been called.

kshegunov

@Suthiro said in QApplication in std::thread:

Nope, it crashes. I think that aboutToQuit() is executed too late, probably in the same time when "ordinary" destructor would have been called.

It is not. It's called before QCoreApplication::exec returns, so I continue to suspect you have an unrelated crash, for which I reiterate that you should provide a meaningful stack trace.

JKSH

@Suthiro said in QApplication in std::thread:

Nope, it crashes.

Hmm, interesting.

Stack trace:

000007fed396b168()
kernel32.dll!000000007702556d()
ntdll.dll!000000007718372d()

Can you please try to get a stack trace again, with a Debug build of your library (with a Debug build of Qt) and with QObject::connect(&a, &QApplication::aboutToQuit, m_qPlotsControl, &QWidget::deleteLater); ?

Suthiro

@kshegunov

It is not. It's called before QCoreApplication::exec returns, so I continue to suspect you have an unrelated crash, for which I reiterate that you should provide a meaningful stack trace.

@JKSH

Can you please try to get a stack trace again, with a Debug build of your library (with a Debug build of Qt) and with QObject::connect(&a, &QApplication::aboutToQuit, m_qPlotsControl, &QWidget::deleteLater); ?

could you please guide me how exactly should I get meaningful stack trace? I'm already using the debug version of Qt.

I'm trying to isolate the crash, but looks like I need to start a fresh project. It could take some time.

Suthiro

Okay, I figured it out how to reproduce the crash without lua.exe. The true culprit is static linking with Qt libraries: only if Qt was compiled with -static switch crash will occur.

So, to reproduce:

Build Qt with -static switch. To be exact, I used
-skip qt3d -skip qtactiveqt -skip qtandroidextras -skip qtconnectivity -skip qtdatavis3d -skip qtdeclarative -skip qtgamepad -skip qtgraphicaleffects -skip qtimageformats -skip qtlocation -skip qtlottie -skip qtmacextras -skip qtmultimedia -skip qtnetworkauth -skip qtpurchasing -skip qtquick3d -skip qtquickcontrols -skip qtquickcontrols2 -skip qtquicktimeline -skip qtremoteobjects -skip qtscript -skip qtsensors -skip qtserialbus -skip qtserialport -skip qtspeech -skip qttranslations -skip qtvirtualkeyboard -skip qtwayland -skip qtwebchannel -skip qtwebengine -skip qtwebglplugin -skip qtwebsockets -skip qtwebview -skip qtx11extras -no-appstore-compliant -no-sql-sqlite -no-sql-sqlite2 -no-sql-psql -no-sql-mysql -no-sql-odbc -no-sql-oci -no-sql-ibase -no-sql-db2 -no-sql-tds -no-dbus -no-icu -debug-and-release -nomake examples -nomake tests -opensource -confirm-license -mp -no-feature-sql -no-feature-testlib -static -prefix C:\temp\qt-test\qt5
Build a dynamically linked library using QtPlotter.h and QtPlotter.cpp.
Build an application using main.cpp. Please note that if the library is linked with the application at compile-time (regardless statically or dynamically), crash does not occur.
Launch the application and interact with the button.
5a. Close the window and enter any symbol except 's' -> crash occurs.
5b. Do not close the window but enter 's' -> no crash.

Application
main.cpp

#include <Windows.h>
#include <iostream>
#include "QtPlotter.h"

int main()
{
	HMODULE qtlib = LoadLibrary(L"QtWidgetsApplication2.dll");
	if (nullptr == qtlib) 
		return EXIT_FAILURE;
	auto start = (void(*)())GetProcAddress(qtlib, "Start");
	if (nullptr != start)
		start();
	else
		return EXIT_FAILURE;
		
	char ch=0;
	std::cout << "enter 's' to exit correctly" << std::endl;
	std::cin >> ch;
	if (ch == 's')
	{
		auto stop = (void(*)())GetProcAddress(qtlib, "Stop");
		if (nullptr != stop)
			stop();
		else
			return EXIT_FAILURE;
	}
	if (nullptr != qtlib)
		FreeLibrary(qtlib);
	else
		return EXIT_FAILURE;
}

Library
QtPlotter.h

#pragma once
#include <memory>
#include <thread>

#ifdef BuildDll
#include <QMainWindow>
#include <QPushButton>
class QPlotsControl : public QMainWindow
{
	Q_OBJECT

public:
	QPlotsControl(QWidget *parent = nullptr) : QMainWindow(parent)
	{m_Btn = new QPushButton(this);}
private:
	QPushButton* m_Btn;
};

class QtPlotter
{
public:
	QtPlotter();
	~QtPlotter();
	void Close();
private:
	void ApplicationLoop();
	std::unique_ptr<std::thread> m_guiThread;
};
#endif

#ifdef BuildDll
#define DllImportExport extern "C" __declspec( dllexport )
#else
#define DllImportExport extern "C" __declspec( dllimport )
#endif

DllImportExport void Start();
DllImportExport void Stop();

QtPlotter.cpp

#include "QtPlotter.h"
#include <QApplication>

QtPlotter* qtPlotter = nullptr;

void QtPlotter::ApplicationLoop()
{
	char* argv = new char[4]{ '\0' };
	auto val = "gui";
	strcpy_s(argv, 4, val);
	int argc = 1;
	QApplication a(argc, &argv);
	auto m_qPlotsControl = new QPlotsControl();
	m_qPlotsControl->show();
	//QObject::connect(&a, &QApplication::aboutToQuit, m_qPlotsControl, &QWidget::deleteLater);
	a.exec();
	delete[] argv;
	return;
}

QtPlotter::QtPlotter()
{
	m_guiThread.reset(new std::thread([this] { this->ApplicationLoop(); }));
}

QtPlotter::~QtPlotter()
{
	m_guiThread->join();
}

void QtPlotter::Close()
{
	QMetaObject::invokeMethod(qApp, []
	{
		for (auto w : qApp->topLevelWidgets())
			w->deleteLater();

	}, Qt::BlockingQueuedConnection);
}

void Start()
{
	if (nullptr == qtPlotter)
		qtPlotter = new QtPlotter();
}

void Stop()
{
	if (nullptr != qtPlotter)
	{
		qtPlotter->Close();
		delete qtPlotter;
		qtPlotter = nullptr;
	}
}

JKSH

@Suthiro said in QApplication in std::thread:

Okay, I figured it out how to reproduce the crash without lua.exe. The true culprit is static linking with Qt libraries: only if Qt was compiled with -static switch crash will occur.

...

Please note that if the library is linked with the application at compile-time (regardless statically or dynamically), crash does not occur.

Thanks again for your detailed tests and reports.

Unfortunately, static libraries and LoadLibrary() are outside my expertise; I can't think of a reason why static Qt or LoadLibrary() would produce different behaviour compared to dynamic Qt or compile-time linking.

By the way, what license are you using Qt under? I you want to use LGPLv3, use statically-linked Qt, AND keep your software closed-source, you'll need to take extra steps to allow your users to switch the version of Qt. (No extra steps needed if your software is open-source)

I use MSVC 2017 (toolset v141) on WIndows 7 x64. Compiled qt5.14.1 from source, but also tried some pre-built binaries.

Hmm... the official pre-built binaries are dynamic only. Just to confirm: Does this mean that dynamic Qt crashes from Lua but doesn't crash from your test app?

could you please guide me how exactly should I get meaningful stack trace? I'm already using the debug version of Qt.

...

Build Qt with -static switch. To be exact, I used...

Add the -developer-build switch when building Qt.
- For some types of debugging, the regular Debug build is detailed enough.
- The Developer build exports even more debugging symbols from Qt compared to the regular Debug build -- see https://wiki.qt.io/Building_Qt_5_from_Git
Build QtPlotter and link it to the developer build of Qt.
Build your test app.
Use a Debugger to launch the test app (or your Lua app)
- I've found that WinDbg provides more details than GDB
Let your app crash and obtain a detailed stack trace from the debugger
- If using WinDbg, press 'k' to get the stack trace after the crash

Suthiro

@JKSH

Thanks again for your detailed tests and reports.
Unfortunately, static libraries and LoadLibrary() are outside my expertise; I can't think of a reason why static Qt or LoadLibrary() would produce different behaviour compared to dynamic Qt or compile-time linking.

I don't know either. I just started from scratch and found what is different. It is very strange indeed.

By the way, what license are you using Qt under? I you want to use LGPLv3, use statically-linked Qt, AND keep your software closed-source, you'll need to take extra steps to allow your users to switch the version of Qt. (No extra steps needed if your software is open-source)

It is a private project (hobby-related), so I'm okay with any license that allows me to use Qt for personal purposes :). In case I decide to make it public for some reason (unlikely), it will be definitely open-source. TBH I don't need static linking with Qt, it is matter of habit to keep things tidier. So I'll just switch to dynamic linking.

I use MSVC 2017 (toolset v141) on WIndows 7 x64. Compiled qt5.14.1 from source, but also tried some pre-built binaries.

Hmm... the official pre-built binaries are dynamic only. Just to confirm: Does this mean that dynamic Qt crashes from Lua but doesn't crash from your test app?

I tried to reproduce the behaviour with pre-built binaries to no avail, I believe there are/were more than one reason to crash in the "big" project. I was focused at the "QTimer" message and QPushButton interaction troubles, so it is very likely that I broke something else. But now I can get rid at least of this one and proceed further. The example I provided crashes both using lua and test app given that Qt was linked statically.

Add the -developer-build switch when building Qt.
For some types of debugging, the regular Debug build is detailed enough.
The Developer build exports even more debugging symbols from Qt compared to the regular Debug build -- see https://wiki.qt.io/Building_Qt_5_from_Git
Build QtPlotter and link it to the developer build of Qt.
Build your test app.
Use a Debugger to launch the test app (or your Lua app)
I've found that WinDbg provides more details than GDB
Let your app crash and obtain a detailed stack trace from the debugger
If using WinDbg, press 'k' to get the stack trace after the crash

okay, I'll build with -developer-build and report later.

Thank you very much for your help!

kshegunov

@JKSH said in QApplication in std::thread:

Unfortunately, static libraries and LoadLibrary() are outside my expertise; I can't think of a reason why static Qt or LoadLibrary() would produce different behaviour compared to dynamic Qt or compile-time linking.

@Suthiro said in QApplication in std::thread:

TBH I don't need static linking with Qt, it is matter of habit to keep things tidier.

Nothing tidy about that ...

@Suthiro said in QApplication in std::thread:

The true culprit is static linking with Qt libraries: only if Qt was compiled with -static switch crash will occur.

There's a gazillion global variables in Qt that must be initialized properly for things to work. Have you initialized the plugins? (a.k.a. show us the .pro file of your library)