Bug 346188

Summary:	ECMQmLoader.cpp generated by ecm may cause crash.
Product:	[Frameworks and Libraries] extra-cmake-modules	Reporter:	Weng Xuetian <wengxt>
Component:	general	Assignee:	Alex Merry <alex.merry>
Status:	RESOLVED FIXED
Severity:	normal	CC:	ecm-bugs-null
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Chakra
OS:	Linux
Latest Commit:	http://commits.kde.org/extra-cmake-modules/fb4d91996affdbcd3ffc19c7646fbf77b96bf1e5	Version Fixed In:
Sentry Crash Report:

Description Weng Xuetian 2015-04-14 21:14:09 UTC

When Qt is compiled with debug, there is one assertion enabled:
QCoreApplication::sendEvent: "Cannot send events to objects owned by a different thread"

In my case, sddm-greeter crashes because plasma qml plugin linked to some library with generated ECMQmLoader.cpp. sddm-greeter load the plugin in qml thread instead of main thread.

Because ECMQmLoader.cpp calls to QCoreApplication::instance()->installTranslator which can be only called from main thread, thus it breaks the assertion above.

Reproducible: Always

Steps to Reproduce:
1. compile qt with -debug
2. run sddm with breeze


Actual Results:  
sddm-greeter crashes.

Expected Results:  
sddm-greeter runs normally.

Comment 1 Alex Merry 2015-05-11 14:56:24 UTC

We use Q_COREAPP_STARTUP_FUNCTION, which will invoke the function immediately if QCoreApplication already exists. C++11 allows global object constructors (which is what Q_COREAPP_STARTUP_FUNCTION uses to do its work) to run in parallel on different threads. Qt does some locking internally, but only enough to make sure multiple functions wrapped with Q_COREAPP_STARTUP_FUNCTION don't get executed simultaneously.

Internally, QCoreApplication::installTranslator uses QCoreApplication::sendEvent, which expects to be called on the QCoreApplication thread (hence the assert).

We probably want to check whether we're running on the QCoreApplication instance's thread and, if not, set up some QObject do to the work, move it to the QCoreApplication thread and use QMetaObject::invokeMethod to request the load.

Comment 2 Alex Merry 2015-05-11 18:35:28 UTC

https://git.reviewboard.kde.org/r/123726/

Comment 3 Alex Merry 2015-11-03 10:24:17 UTC

Git commit 009c480413910e8c1a18f4d1420f4a517ea606e6 by Alex Merry.
Committed on 03/11/2015 at 10:22.
Pushed by alexmerry into branch 'master'.

Make sure we load translations on the main thread.
REVIEW: 123726

M  +13   -2    modules/ECMPoQmTools.cmake
M  +63   -27   modules/ECMQmLoader.cpp.in
M  +2    -2    tests/CMakeLists.txt
M  +27   -5    tests/ECMPoQmToolsTest/CMakeLists.txt
R  +30   -10   tests/ECMPoQmToolsTest/check.cmake.in [from: tests/ECMPoQmToolsTest/check_tree.cmake.in - 052% similarity]
A  +2    -0    tests/ECMPoQmToolsTest/check_conf.cmake.in
A  +22   -0    tests/ECMPoQmToolsTest/tr_test-po/en/catalog.po
A  +22   -0    tests/ECMPoQmToolsTest/tr_test-po/en_GB/catalog.po
A  +16   -0    tests/ECMPoQmToolsTest/tr_test.cpp     [License: UNKNOWN]  *
A  +68   -0    tests/ECMPoQmToolsTest/tr_thread_test.cpp     [License: UNKNOWN]  *
A  +12   -0    tests/ECMPoQmToolsTest/tr_thread_test_module.cpp     [License: UNKNOWN]  *

The files marked with a * at the end have a non valid license. Please read: http://techbase.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page.


http://commits.kde.org/extra-cmake-modules/009c480413910e8c1a18f4d1420f4a517ea606e6

Comment 4 Alex Merry 2015-11-08 19:31:40 UTC

Git commit 6745bd7e4796560959bb67e33b7c7f86f96a5a94 by Alex Merry.
Committed on 08/11/2015 at 19:29.
Pushed by alexmerry into branch 'master'.

Revert "Make sure we load translations on the main thread."

This broke the build for projects which used ecm_create_qm_loader in
unusual ways. A better approach is coming, but won't be in e-c-m 5.16.

This reverts commit 009c480413910e8c1a18f4d1420f4a517ea606e6.
CCMAIL: release-team@kde.org
CCMAIL: kde-buildsystem@kde.org

M  +2    -13   modules/ECMPoQmTools.cmake
M  +26   -62   modules/ECMQmLoader.cpp.in
M  +2    -2    tests/CMakeLists.txt
M  +5    -27   tests/ECMPoQmToolsTest/CMakeLists.txt
D  +0    -2    tests/ECMPoQmToolsTest/check_conf.cmake.in
R  +10   -30   tests/ECMPoQmToolsTest/check_tree.cmake.in [from: tests/ECMPoQmToolsTest/check.cmake.in - 052% similarity]
D  +0    -22   tests/ECMPoQmToolsTest/tr_test-po/en/catalog.po
D  +0    -22   tests/ECMPoQmToolsTest/tr_test-po/en_GB/catalog.po
D  +0    -16   tests/ECMPoQmToolsTest/tr_test.cpp
D  +0    -68   tests/ECMPoQmToolsTest/tr_thread_test.cpp
D  +0    -12   tests/ECMPoQmToolsTest/tr_thread_test_module.cpp

http://commits.kde.org/extra-cmake-modules/6745bd7e4796560959bb67e33b7c7f86f96a5a94

Comment 5 Alex Merry 2015-11-08 19:36:09 UTC

Sorry, this broke builds, so I'm going to have to revisit the fix.

Comment 6 Alex Merry 2015-11-30 14:07:51 UTC

Git commit fb4d91996affdbcd3ffc19c7646fbf77b96bf1e5 by Alex Merry.
Committed on 30/11/2015 at 14:07.
Pushed by alexmerry into branch 'master'.

Make sure we load translations on the main thread.

Because the old implementation (accidentally) worked when you put the
ecm_create_qm_loader call in a different CMakeLists.txt file to the
target the file was added to, some projects did this.

This won't work with build-time-generated files, though, like moc files.
So we (ab)use QTimer events to make the loading happen on the main
thread.
REVIEW: 126000

M  +12   -5    modules/ECMPoQmTools.cmake
M  +67   -26   modules/ECMQmLoader.cpp.in
M  +42   -2    tests/ECMPoQmToolsTest/CMakeLists.txt
M  +3    -0    tests/ECMPoQmToolsTest/check.cmake.in
M  +2    -0    tests/ECMPoQmToolsTest/check_conf.cmake.in
A  +69   -0    tests/ECMPoQmToolsTest/tr_thread_test.cpp     [License: UNKNOWN]  *
A  +12   -0    tests/ECMPoQmToolsTest/tr_thread_test_module.cpp     [License: UNKNOWN]  *

The files marked with a * at the end have a non valid license. Please read: http://techbase.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page.


http://commits.kde.org/extra-cmake-modules/fb4d91996affdbcd3ffc19c7646fbf77b96bf1e5