Skip to content

fix(CustomPlugin): guard the QML engine pointer through shutdown - #14755

Open
TSC21 wants to merge 2 commits into
mavlink:masterfrom
TSC21:fix/custom-plugin-shutdown-use-after-free
Open

fix(CustomPlugin): guard the QML engine pointer through shutdown#14755
TSC21 wants to merge 2 commits into
mavlink:masterfrom
TSC21:fix/custom-plugin-shutdown-use-after-free

Conversation

@TSC21

@TSC21 TSC21 commented Jul 31, 2026

Copy link
Copy Markdown
Member

RIIS

Contributed on behalf of RIIS, LLC.



Bug Description

Every exit of a build carrying the custom plugin (QGC_CUSTOM_DIR=custom-example, or any downstream custom build based on the template) ends in a SIGSEGV during shutdown. The app's work completes normally and the process then crashes on the way out. It also fails core tests that drive the full application lifecycle when they run against a custom build: MissionCommandTreeEditorTest and VehicleLinkManagerTest pass all their cases and then die in the shutdown crash, which CTest reports as a failure.

Root Cause

The application owns the QML engine and destroys it before the plugin, so shutdown reaches CustomPlugin::cleanup() with _qmlEngine pointing at a destroyed engine, and removeUrlInterceptor() is called through a raw pointer into freed memory. The existing null check does not help because the pointer never becomes null. gdb backtrace:

#0 QQmlEngine::removeUrlInterceptor(QQmlAbstractUrlInterceptor*)
#1 CustomPlugin::cleanup() at custom-example/src/CustomPlugin.cc:60
#2 QGCApplication::shutdown()
#3 main()

Solution

Hold the engine as a QPointer<QQmlApplicationEngine>. It nulls itself when the engine is destroyed, so the existing guard in cleanup() finally answers the question it asks. No behaviour change on any path where the engine is still alive.

Testing

  • Tested locally (custom build exits cleanly; MissionCommandTreeEditorTest and VehicleLinkManagerTest go from shutdown-crash to passing)
  • Added regression test (the two existing full-lifecycle tests above already catch this whenever they run against a custom build)
  • Tested with simulator (SITL)
  • Tested with hardware

Platforms Tested

  • Linux
  • Windows
  • macOS
  • Android
  • iOS

Flight Stacks Tested

  • PX4
  • ArduPilot
  • N/A

Checklist

  • I have read the Contribution Guidelines
  • My code follows the project's coding standards
  • I have added a test that reproduces the bug (it crashes in removeUrlInterceptor without the fix)
  • New and existing unit tests pass locally

By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).

The application owns the QML engine and destroys it ahead of the
plugin, so shutdown reaches CustomPlugin::cleanup() with the engine
already gone and calls removeUrlInterceptor() through a raw pointer
into freed memory. Every exit of a build carrying the custom plugin
takes this path: the run's work completes and the process then dies
on SIGSEGV. It also fails any test that drives the full application
lifecycle against a custom build (MissionCommandTreeEditorTest and
VehicleLinkManagerTest end in the crash after all their cases pass).

Holding the engine as a QPointer makes the existing null guard in
cleanup() answer the question it asks: the pointer nulls itself when
the engine is destroyed.

Verified with a gdb backtrace pinning the crash to cleanup() calling
QQmlEngine::removeUrlInterceptor() after engine destruction, and by
MissionCommandTreeEditorTest and VehicleLinkManagerTest passing on a
custom build with the guard in place.
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ Build results unavailable — artifact download from one or more platform workflows failed (likely artifact retention expiry or transient API error). The combined report cannot be generated for this run.

See the Build Results workflow run for details.

The application destroys the QML engine ahead of the plugin, so
cleanup() always runs after the engine is gone; the QPointer guard
is what keeps that from being a use-after-free, and nothing pinned
it. The case drives the real creation path, destroys the engine the
way shutdown ordering does, and cleans up twice; reaching the end of
the case is the property, because with a raw engine handle the first
cleanup dies inside QQmlEngine::removeUrlInterceptor.

The double call is deliberate: shutdown owns one call, and cleanup
must stay a no-op for any second arrival, so the interceptor handle
nulls itself after the delete. The test subdirectory rides the same
CUSTOM_SOURCES cache the overlay already hands to the application
target and compiles only under QGC_BUILD_TESTING, so a production
overlay build carries no test code.
@TSC21
TSC21 requested a review from HTRamsey as a code owner July 31, 2026 19:07
@TSC21

TSC21 commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Added a regression test on top of the fix: CustomPluginTest drives the real engine creation path, deletes the engine the way shutdown ordering does, and calls cleanup() twice. Without the QPointer guard the first cleanup dies inside QQmlEngine::removeUrlInterceptor, so the case reproduces the crash exactly; the second call pins cleanup as re-entrant, with the interceptor handle nulled after delete. The test rides the custom-example CUSTOM_SOURCES hook and only compiles under QGC_BUILD_TESTING. Verified green on a Linux custom build (Qt 6.11.1): the case passes in 0.5 s and the crash returns if the guard is reverted.

@github-actions github-actions Bot added the CMake label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant