diff --git a/custom-example/CMakeLists.txt b/custom-example/CMakeLists.txt index fad3246fc61c..7599d5d23aa8 100644 --- a/custom-example/CMakeLists.txt +++ b/custom-example/CMakeLists.txt @@ -152,3 +152,10 @@ set(CUSTOM_QT_COMPONENTS Core CACHE INTERNAL "" FORCE ) + +# Compiled into the application target and registered with CTest, but only when testing is +# enabled, so a production overlay build carries no test code. The subdirectory appends to the +# same CUSTOM_SOURCES cache src/CMakeLists.txt hands to the application target. +if(QGC_BUILD_TESTING AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/test/CMakeLists.txt") + add_subdirectory(test) +endif() diff --git a/custom-example/src/CustomPlugin.cc b/custom-example/src/CustomPlugin.cc index b0c5283b54df..0f2c93d44261 100644 --- a/custom-example/src/CustomPlugin.cc +++ b/custom-example/src/CustomPlugin.cc @@ -52,7 +52,10 @@ void CustomPlugin::cleanup() _qmlEngine->removeUrlInterceptor(_selector); } + // Nulled so a second cleanup is a no-op: shutdown owns one call, and the regression test that + // pins the engine guard above exercises the call twice on purpose. delete _selector; + _selector = nullptr; } void CustomPlugin::_advancedChanged(bool changed) diff --git a/custom-example/src/CustomPlugin.h b/custom-example/src/CustomPlugin.h index fe0b4e88a290..2de94f6a65a1 100644 --- a/custom-example/src/CustomPlugin.h +++ b/custom-example/src/CustomPlugin.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -91,7 +92,12 @@ private slots: void _addSettingsEntry(const QString& title, const char* qmlFile, const char* iconFile = nullptr); CustomOptions *_options = nullptr; - QQmlApplicationEngine *_qmlEngine = nullptr; + + /// Guarded, because the engine is owned by the application and dies before this plugin does: + /// shutdown reaches cleanup() with the engine already destroyed, and a raw pointer there is a + /// use-after-free on every exit. The guard nulls itself with the engine. + QPointer _qmlEngine; + class CustomOverrideInterceptor *_selector = nullptr; QVariantList _customSettingsList; // Not to be mixed up with QGCCorePlugin implementation }; diff --git a/custom-example/test/CMakeLists.txt b/custom-example/test/CMakeLists.txt new file mode 100644 index 000000000000..8d0a0a4f715f --- /dev/null +++ b/custom-example/test/CMakeLists.txt @@ -0,0 +1,33 @@ +# Custom plugin unit tests. The .cc/.h pairs are appended to the CUSTOM_SOURCES cache that +# src/CMakeLists.txt hands to the application target, so AUTOMOC and the test-framework include +# directories the application already carries under QGC_BUILD_TESTING apply with no extra wiring. +# Registration with CTest is done directly: this directory is processed before the application +# target exists, so add_qgc_test()'s check-target dependencies are not available yet, and +# add_test() resolves the binary through a generator expression at generate time. + +if(NOT QGC_BUILD_TESTING) + return() +endif() + +list(APPEND CUSTOM_SOURCES + ${CMAKE_CURRENT_SOURCE_DIR}/CustomPluginTest.cc + ${CMAKE_CURRENT_SOURCE_DIR}/CustomPluginTest.h +) +set(CUSTOM_SOURCES ${CUSTOM_SOURCES} CACHE INTERNAL "" FORCE) + +list(APPEND CUSTOM_INCLUDE_DIRECTORIES + ${CMAKE_CURRENT_SOURCE_DIR} +) +set(CUSTOM_INCLUDE_DIRECTORIES ${CUSTOM_INCLUDE_DIRECTORIES} CACHE INTERNAL "" FORCE) + +add_test( + NAME CustomPluginTest + COMMAND $ --unittest:CustomPluginTest --allow-multiple + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} +) + +set_tests_properties(CustomPluginTest PROPERTIES + TIMEOUT 60 + LABELS "Unit" + ENVIRONMENT "QT_QPA_PLATFORM=offscreen;QT_QUICK_BACKEND=software;LIBGL_ALWAYS_SOFTWARE=1" +) diff --git a/custom-example/test/CustomPluginTest.cc b/custom-example/test/CustomPluginTest.cc new file mode 100644 index 000000000000..e72a4cbcb015 --- /dev/null +++ b/custom-example/test/CustomPluginTest.cc @@ -0,0 +1,25 @@ +#include "CustomPluginTest.h" + +#include + +#include "QGCCorePlugin.h" + +void CustomPluginTest::_cleanupSurvivesEngineDestruction_test() +{ + QGCCorePlugin* const plugin = QGCCorePlugin::instance(); + QVERIFY(plugin != nullptr); + + // The real creation path: the engine the plugin will hold a handle to. + QQmlApplicationEngine* const engine = plugin->createQmlApplicationEngine(nullptr); + QVERIFY(engine != nullptr); + + // The application destroys the engine ahead of the plugin, so cleanup() always runs against a + // destroyed engine. Reaching the end of this case IS the property: with a raw engine handle + // the first cleanup dies in QQmlEngine::removeUrlInterceptor on freed memory. + delete engine; + + plugin->cleanup(); + plugin->cleanup(); +} + +UT_REGISTER_TEST(CustomPluginTest, TestLabel::Unit) diff --git a/custom-example/test/CustomPluginTest.h b/custom-example/test/CustomPluginTest.h new file mode 100644 index 000000000000..bfb8144d2c6c --- /dev/null +++ b/custom-example/test/CustomPluginTest.h @@ -0,0 +1,16 @@ +#pragma once + +#include "UnitTest.h" + +/// Pins the plugin's shutdown contract. The application owns the QML engine and destroys it ahead +/// of the plugin, so cleanup() always runs after the engine is gone; a plugin that keeps a raw +/// pointer to it turns every exit into a use-after-free. The case drives the real creation path, +/// destroys the engine the way shutdown ordering does, and then cleans up twice, because shutdown +/// owns one call and nothing may break if another arrives. +class CustomPluginTest : public UnitTest +{ + Q_OBJECT + +private slots: + void _cleanupSurvivesEngineDestruction_test(); +};