Crash on Location display toggle during template load

[lpechacek]

How to replicate:

1. Open a map with many templates
2. It takes some time to load them all
3. Switch Location display toggle on during the template load
4. OOM crashes

Originally posted by @ollesmaps in #1892 (comment)

Observed with OpenOrienteering-Mapper-refs_pull_1738_merge_v20210207.1-Android-armeabi-v7a test build.

The backtrace is always as this:

    #00 pc 000c0250  /data/app/org.openorienteering.mapper.merge-KuSUUHJD60P_Q7NLBkGjsQ==/lib/arm/libQt5Core.so (QString::arg(QString const&, int, QChar) const+4)
    #01 pc 0015e853  /data/app/org.openorienteering.mapper.merge-KuSUUHJD60P_Q7NLBkGjsQ==/lib/arm/libMapper.so
    #02 pc 001638e5  /data/app/org.openorienteering.mapper.merge-KuSUUHJD60P_Q7NLBkGjsQ==/lib/arm/libQt5Core.so
    #03 pc 0015c35d  /data/app/org.openorienteering.mapper.merge-KuSUUHJD60P_Q7NLBkGjsQ==/lib/arm/libQt5Core.so (QObject::event(QEvent*)+64)
    #04 pc 00107a2f  /data/app/org.openorienteering.mapper.merge-KuSUUHJD60P_Q7NLBkGjsQ==/lib/arm/libQt5Widgets.so (QApplicationPrivate::notify_helper(QObject*, QEvent*)+152)

Unfortunately, I've deleted the corresponding .apk from my storage, so cannot proceed with reliable analysis. I'm waiting for replication with an official build.

[lpechacek]

So, I have all the necessary bits. Too bad that I don't know where to get debuginfo for the build artifacts, but still I made progress.

AFAICT, the crash originates at map.cpp:1990.

mapper/src/core/map.cpp

Lines 1979 to 1999 in ac4b084

	void Map::loadTemplateFilesAsync(MapView& view, std::function<void(const QString&)> listener)
	{
	(void)listener; // fix false-positive warning from misc-unused-parameters
	for (auto& temp : templates)
	{
	if (temp->getTemplateState() == Template::Unloaded
	&& view.getTemplateVisibility(temp.get()).visible)
	{
	QTimer::singleShot(10, temp.get(), ([this, &view, &temp, log = std::move(listener)]() {
	log(qApp->translate("OpenOrienteering::MainWindow", "Opening %1")
	.arg(temp->getTemplateFilename()));
	if (temp->getTemplateState() != Template::Loaded)
	temp->loadTemplateFile();
	log(QString{});
	loadTemplateFilesAsync(view, std::move(log));
	}));
	return;
	}
	}
	}

[dg0yt]

Between template selection and template loading, this code leaves 10 ms for event processing, meant for user input and display updates. This is a potential gap for a race condition (like TOCTOU), e.g. when the function is invoked a second time while an event is already pending.

The lambda called by singleShot is bound to the template object, i.e. when the template is deleted, Qt shall remove the scheduled call from the event loop, thus preventing the access of deleted templates. If this doesn't happen immediately, then it will break.

[dg0yt]

I tried to simulate this behaviour on the desktop, but failed. I did the the following:

• Added an extra QThread::sleep(1000) to the lambda function in above code, to simulate slow loading.
• Switched to touch mode.
• Selected the fake position source (available in developer build).
• Opened a map with multiple GPX templates.
• Clicked "Enable GPS display" during template loading.

[dg0yt]

Extra note: with the current 10 ms gap for event processing, input events don't seemt to be handled as soon as expected. With 50 ms, I can toggle GPS display multiple times during template loading.

[dg0yt]

Looks like I arrive at reproducing in Android Emulator. Still need to arrange a full debug environment.

[dg0yt]

Still no good debugging on Android, but now also reproducible on desktop. A simple test file was not enough. I have the same GPX file app. 10 times below and 10 times above the map.

[dg0yt]

The lambda called by singleShot is bound to the template object, i.e. when the template is deleted, Qt shall remove the scheduled call from the event loop, thus preventing the access of deleted templates. If this doesn't happen immediately, then it will break.

It turned out that the problem was the lifetime of unique_ptr in the template container, not the lifetime of the template.