QML app fix: crash closing, notification deadlock

The crash was introduced at 533dbef0c7

This has been particurarly tricky as lot of different parts contributed
  in causing unexpected behaviours

When the activity is created onNewIntent is not called and we have to
  get the intent data from C++ bu other means, but C++ code is running
  in a different thread so there is no guarantee that the intent data is
  reacheable yet on starting, so the C++ code has to wait for the intent
  data being ready, but paying attention to not cause a deadlock beetween
  the two thread (the android ui thread may be waiting for some
  operation performed by Qt)

Because of notification intent flags not properly set the activity was
  recreated also if it was already on top, this caused a nasty
  interaction between android ui thread and qt thread that derived in a
  deadlock, to avoid this lot of try/error has been made until the
  proper soup of manifest and intent flags has been found

At this point link handling, notification handling, and Activity closing
  should work as expected without any deadlock or crash

retroshare-android-notify-service/src/notificationsbridge.h

@@ -22,8 +22,8 @@
 #include <QDebug>
 
 #ifdef __ANDROID__
-    #include <QtAndroid>
+#	include <QtAndroid>
-    #include <QtAndroidExtras/QAndroidJniObject>
+#	include <QtAndroidExtras/QAndroidJniObject>
 #endif // __ANDROID__
 
 struct NotificationsBridge : QObject

retroshare-qml-app/src/android/android.iml

@@ -64,14 +64,6 @@
       <sourceFolder url="file://$MODULE_DIR$/java" isTestSource="false" />
       <sourceFolder url="file://$MODULE_DIR$/src/main/jni" isTestSource="false" />
       <sourceFolder url="file://$MODULE_DIR$/src/main/shaders" isTestSource="false" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/res" type="java-test-resource" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/resources" type="java-test-resource" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/assets" type="java-test-resource" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/aidl" isTestSource="true" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/java" isTestSource="true" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/jni" isTestSource="true" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/rs" isTestSource="true" />
-      <sourceFolder url="file://$MODULE_DIR$/src/test/shaders" isTestSource="true" />
       <sourceFolder url="file://$MODULE_DIR$/src/androidTest/res" type="java-test-resource" />
       <sourceFolder url="file://$MODULE_DIR$/src/androidTest/resources" type="java-test-resource" />
       <sourceFolder url="file://$MODULE_DIR$/src/androidTest/assets" type="java-test-resource" />
@@ -80,6 +72,14 @@
       <sourceFolder url="file://$MODULE_DIR$/src/androidTest/jni" isTestSource="true" />
       <sourceFolder url="file://$MODULE_DIR$/src/androidTest/rs" isTestSource="true" />
       <sourceFolder url="file://$MODULE_DIR$/src/androidTest/shaders" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/res" type="java-test-resource" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/resources" type="java-test-resource" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/assets" type="java-test-resource" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/aidl" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/java" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/jni" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/rs" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/src/test/shaders" isTestSource="true" />
       <excludeFolder url="file://$MODULE_DIR$/.build/intermediates/blame" />
       <excludeFolder url="file://$MODULE_DIR$/.build/intermediates/incremental" />
       <excludeFolder url="file://$MODULE_DIR$/.build/intermediates/manifests" />

retroshare-qml-app/src/android/src/NativeCalls.cpp

@@ -17,30 +17,27 @@
  */
 
 #include "NativeCalls.h"
-#include "singletonqmlengine.h"
+#include "rsqmlappengine.h"
 
-#include <QString>
-#include <QQmlApplicationEngine>
 #include <QMetaObject>
 #include <QDebug>
-#include <QQuickWindow>
 
 JNIEXPORT void JNICALL
 Java_org_retroshare_android_qml_1app_jni_NativeCalls_notifyIntentUri
 (JNIEnv* env, jclass, jstring uri)
 {
+	qDebug() << __PRETTY_FUNCTION__;
+
 	const char *uriBytes = env->GetStringUTFChars(uri, NULL);
 	QString uriStr(uriBytes);
 	env->ReleaseStringUTFChars(uri, uriBytes);
 
-	QQmlApplicationEngine& engine(SingletonQmlEngine::instance());
+	RsQmlAppEngine* engine = RsQmlAppEngine::mainInstance();
-	QObject* rootObj = engine.rootObjects()[0];
+	if(engine)
-	QQuickWindow* mainWindow = qobject_cast<QQuickWindow*>(rootObj);
+		QMetaObject::invokeMethod(
-
+		            engine, "handleUri",
-	if(mainWindow)
+		            Qt::QueuedConnection, // BlockingQueuedConnection, AutoConnection
-	{
+		            Q_ARG(QString, uriStr));
-		QMetaObject::invokeMethod(mainWindow, "handleIntentUri",
+	else qCritical() << __PRETTY_FUNCTION__ << "RsQmlAppEngine::mainInstance()"
-		                          Q_ARG(QVariant, uriStr));
+	                 << "not initialized yet!";
-	}
-	else qCritical() << __PRETTY_FUNCTION__ << "Root object is not a window!";
 }

retroshare-qml-app/src/android/src/org/retroshare/android/qml_app/RetroShareAndroidNotifyService.java

@@ -21,7 +21,6 @@ package org.retroshare.android.qml_app;
 import android.app.Notification;
 import android.app.NotificationManager;
 import android.app.PendingIntent;
-import android.app.TaskStackBuilder;
 import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
@@ -39,19 +38,23 @@ public class RetroShareAndroidNotifyService extends QtService
 				.setContentText(text)
 				.setAutoCancel(true);
 
-		Intent resultIntent = new Intent(this, RetroShareQmlActivity.class);
+		Intent intent = new Intent(this, RetroShareQmlActivity.class);
-		if(!uri.isEmpty()) resultIntent.setData(Uri.parse(uri));
+		intent.setFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP);
+		intent.addFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP);
 
-		TaskStackBuilder stackBuilder = TaskStackBuilder.create(this);
+		if(!uri.isEmpty()) intent.setData(Uri.parse(uri));
-		stackBuilder.addParentStack(RetroShareQmlActivity.class);
-		stackBuilder.addNextIntent(resultIntent);
-		PendingIntent resultPendingIntent =
-				stackBuilder.getPendingIntent( 0,
-						PendingIntent.FLAG_UPDATE_CURRENT );
 
-		mBuilder.setContentIntent(resultPendingIntent);
+		PendingIntent pendingIntent = PendingIntent.getActivity(
+				this, NOTIFY_REQ_CODE, intent,
+				PendingIntent.FLAG_UPDATE_CURRENT);
+
+		mBuilder.setContentIntent(pendingIntent);
 		NotificationManager mNotificationManager =
 				(NotificationManager) this.getSystemService(Context.NOTIFICATION_SERVICE);
 		mNotificationManager.notify(0, mBuilder.build());
 	}
+
+	/** Must not be 0 otherwise a new activity may be created when should not
+	 * (ex. the activity is already visible/on top) and deadlocks happens */
+	private static final int NOTIFY_REQ_CODE = 2173;
 }

retroshare-qml-app/src/android/src/org/retroshare/android/qml_app/RetroShareQmlActivity.java

@@ -33,6 +33,8 @@ public class RetroShareQmlActivity extends QtActivity
 	@Override
 	public void onCreate(Bundle savedInstanceState)
 	{
+		Log.i("RetroShareQmlActivity", "onCreate()");
+
 		if (!isMyServiceRunning(RetroShareAndroidService.class))
 		{
 			Log.i("RetroShareQmlActivity", "onCreate(): RetroShareAndroidService is not running, let's start it by Intent");
@@ -55,13 +57,12 @@ public class RetroShareQmlActivity extends QtActivity
 	@Override
 	public void onNewIntent(Intent intent)
 	{
+		Log.i("RetroShareQmlActivity", "onNewIntent(Intent intent)");
+
 		super.onNewIntent(intent);
+
 		String uri = intent.getDataString();
-		if (uri != null)
+		if (uri != null) NativeCalls.notifyIntentUri(uri);
-		{
-			Log.d("RetroShareQmlActivity", "onNewIntent() " + uri);
-			NativeCalls.notifyIntentUri(uri);
-		}
 	}
 
 	private boolean isMyServiceRunning(Class<?> serviceClass)

retroshare-qml-app/src/main.cpp

@@ -16,15 +16,22 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <QtGlobal>
 #include <QGuiApplication>
 #include <QQmlApplicationEngine>
 #include <QQmlContext>
 #include <QQmlComponent>
 #include <QDebug>
 
+#ifdef Q_OS_ANDROID
+#	include <QtAndroid>
+#	include <QtAndroidExtras/QAndroidJniObject>
+#	include <atomic>
+#endif // Q_OS_ANDROID
+
 #include "libresapilocalclient.h"
 #include "retroshare/rsinit.h"
-#include "singletonqmlengine.h"
+#include "rsqmlappengine.h"
 
 int main(int argc, char *argv[])
 {
@@ -43,17 +50,82 @@ int main(int argc, char *argv[])
 	LibresapiLocalClient rsApi;
 	rsApi.openConnection(sockPath);
 
-	QQmlApplicationEngine& engine(SingletonQmlEngine::instance());
+	RsQmlAppEngine engine(true);
+	QQmlContext& rootContext = *engine.rootContext();
+
+	QStringList mainArgs = app.arguments();
+
+#ifdef Q_OS_ANDROID
+	rootContext.setContextProperty("Q_OS_ANDROID", true);
+
+	/* Add Activity Intent data to args, because onNewIntent is called only if
+	 * the Intet was triggered when the Activity was already created, so only in
+	 * case onCreate is not called.
+	 * The solution exposed in http://stackoverflow.com/a/36942185 is not
+	 * adaptable to our case, because when onCreate is called the RsQmlAppEngine
+	 * is not ready yet.
+	 */
+	uint waitCount = 0;
+	std::atomic<bool> waitIntent(true);
+	QString uriStr;
+	do
+	{
+		QtAndroid::runOnAndroidThread(
+		            [&waitIntent, &uriStr]()
+		{
+			QAndroidJniObject activity = QtAndroid::androidActivity();
+			if(!activity.isValid())
+			{
+				qDebug() << "QtAndroid::runOnAndroidThread(...)"
+				         << "activity not ready yet";
+				return;
+			}
+
+			QAndroidJniObject intent = activity.callObjectMethod(
+			            "getIntent", "()Landroid/content/Intent;");
+			if(!intent.isValid())
+			{
+				qDebug() << "QtAndroid::runOnAndroidThread(...)"
+				         << "intent not ready yet";
+				return;
+			}
+
+			QAndroidJniObject intentData = intent.callObjectMethod(
+			            "getDataString", "()Ljava/lang/String;");
+			if(intentData.isValid()) uriStr = intentData.toString();
+
+			waitIntent = false;
+		});
+
+		if(waitIntent)
+		{
+			qWarning() << "uriStr not ready yet after waiting"
+			           << waitCount << "times";
+			app.processEvents();
+			++waitCount;
+			usleep(10000);
+		}
+	}
+	while (waitIntent);
+
+	qDebug() << "Got uriStr:" << uriStr;
+
+	if(!uriStr.isEmpty()) mainArgs.append(uriStr);
+#else
+	rootContext.setContextProperty("Q_OS_ANDROID", false);
+#endif
+
+	rootContext.setContextProperty("mainArgs", mainArgs);
 
 #ifdef QT_DEBUG
-	engine.rootContext()->setContextProperty("QT_DEBUG", true);
+	rootContext.setContextProperty("QT_DEBUG", true);
 #else
-	engine.rootContext()->setContextProperty("QT_DEBUG", false);
+	rootContext.setContextProperty("QT_DEBUG", false);
 #endif // QT_DEBUG
 
-	engine.rootContext()->setContextProperty("apiSocketPath", sockPath);
+	rootContext.setContextProperty("apiSocketPath", sockPath);
-	engine.rootContext()->setContextProperty("rsApi", &rsApi);
+	rootContext.setContextProperty("rsApi", &rsApi);
-	engine.load(QUrl(QLatin1String("qrc:/main.qml")));
+	engine.load(QUrl(QLatin1String("qrc:/main-app.qml")));
 
 	return app.exec();
 }

retroshare-qml-app/src/main.qml

@@ -35,9 +35,21 @@ ApplicationWindow
 	property bool coreReady: stackView.state === "running_ok" ||
 							 stackView.state === "running_ok_no_full_control"
 
-	Component.onCompleted: addUriHandler("/certificate", certificateLinkHandler)
+	Component.onCompleted:
+	{
+		addUriHandler("/certificate", certificateLinkHandler)
+
+		var argc = mainArgs.length
+		for(var i=0; i<argc; ++i)
+		{
+			var dump = UriJs.URI.parse(mainArgs[i])
+			if(dump.protocol && (dump.query || dump.path))
+				handleIntentUri(mainArgs[i])
+		}
+	}
 
 	property var uriHandlersRegister: ({})
+	property var pendingUriRegister: []
 	function addUriHandler(path, fun) { uriHandlersRegister[path] = fun }
 	function delUriHandler(path, fun) { delete uriHandlersRegister[path] }
 
@@ -124,6 +136,8 @@ ApplicationWindow
 	{
 		id: stackView
 		anchors.fill: parent
+		focus: true
+		onCurrentItemChanged: if (currentItem) currentItem.focus = true
 		Keys.onReleased:
 			if (event.key === Qt.Key_Back && stackView.depth > 1)
 			{
@@ -181,6 +195,9 @@ ApplicationWindow
 						coreStateCheckTimer.stop()
 						stackView.clear()
 						stackView.push("qrc:/Contacts.qml")
+						while(mainWindow.pendingUriRegister.length > 0)
+							mainWindow.handleIntentUri(
+										mainWindow.pendingUriRegister.shift())
 					}
 				}
 			},
@@ -256,7 +273,12 @@ ApplicationWindow
 	{
 		console.log("certificateLinkHandler(uriStr)", coreReady)
 
-		if(!coreReady) return
+		if(!coreReady)
+		{
+			// Save cert uri for later processing as we need core to examine it
+			pendingUriRegister.push(uriStr)
+			return
+		}
 
 		var uri = new UriJs.URI(uriStr)
 		var uQuery = uri.search(true)

retroshare-qml-app/src/retroshare-qml-app.pro

@@ -4,12 +4,16 @@ QT += core network qml quick
 
 CONFIG += c++11
 
-HEADERS += libresapilocalclient.h singletonqmlengine.h
+HEADERS += libresapilocalclient.h \
-SOURCES += main.cpp libresapilocalclient.cpp
+    rsqmlappengine.h
+SOURCES += main-app.cpp \
+    libresapilocalclient.cpp \
+    rsqmlappengine.cpp
 
 RESOURCES += qml.qrc
 
 android-g++ {
+    QT += androidextras
     SOURCES += NativeCalls.cpp
     HEADERS += NativeCalls.h
 }

retroshare-qml-app/src/rsqmlappengine.cpp

@@ -1,6 +1,5 @@
-#pragma once
 /*
- * libresapi local socket client
+ * RetroShare Qml App
  * Copyright (C) 2017  Gioacchino Mazzurco <gio@eigenlab.org>
  *
  * This program is free software: you can redistribute it and/or modify
@@ -17,16 +16,25 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <QQmlApplicationEngine>
+#include "rsqmlappengine.h"
 
-struct SingletonQmlEngine
+#include <QQuickWindow>
+#include <QDebug>
+
+
+/*static*/ RsQmlAppEngine* RsQmlAppEngine::mMainInstance = nullptr;
+
+void RsQmlAppEngine::handleUri(QString uri)
 {
-	static QQmlApplicationEngine& instance()
+	QObject* rootObj = rootObjects()[0];
-	{
+	QQuickWindow* mainWindow = qobject_cast<QQuickWindow*>(rootObj);
-		static QQmlApplicationEngine engine;
-		return engine;
-	}
 
-private:
+	if(mainWindow)
-	SingletonQmlEngine();
+	{
-};
+		QMetaObject::invokeMethod(mainWindow, "handleIntentUri",
+		                          Qt::AutoConnection,
+		                          Q_ARG(QVariant, uri));
+	}
+	else qCritical() << __PRETTY_FUNCTION__
+	                 << "Root object is not a window!";
+}

retroshare-qml-app/src/rsqmlappengine.h

@@ -0,0 +1,67 @@
+#pragma once
+/*
+ * RetroShare Qml App
+ * Copyright (C) 2017  Gioacchino Mazzurco <gio@eigenlab.org>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <QObject>
+#include <QQmlApplicationEngine>
+
+
+class RsQmlAppEngine : public QQmlApplicationEngine
+{
+	Q_OBJECT
+
+public:
+
+	RsQmlAppEngine(bool isMainInstance = false, QObject* parent = nullptr) :
+	    QQmlApplicationEngine(parent)
+	{ if(isMainInstance) mMainInstance = this; }
+
+	~RsQmlAppEngine() { if(mMainInstance == this) mMainInstance = nullptr; }
+
+	static RsQmlAppEngine* mainInstance() { return mMainInstance; }
+
+public slots:
+	void handleUri(QString uri);
+
+private:
+
+	/**
+	 * Using a static variable as QQmlApplicationEngine singleton caused crash
+	 * on application termination, while using a dinamically allocated one
+	 * causes deadlock on termination in QML WorkerScript destructor object
+	 * with the following stacktrace
+	 * #0  0x00007f0a8c3e4c67 in sched_yield () from /lib64/libc.so.6
+	 * #1  0x00007f0a8db5191c in QQuickWorkerScriptEngine::~QQuickWorkerScriptEngine() () from /usr/lib64/libQt5Qml.so.5
+	 * #2  0x00007f0a8db51949 in QQuickWorkerScriptEngine::~QQuickWorkerScriptEngine() () from /usr/lib64/libQt5Qml.so.5
+	 * #3  0x00007f0a8d61359c in QObjectPrivate::deleteChildren() () from /usr/lib64/libQt5Core.so.5
+	 * #4  0x00007f0a8d61ab83 in QObject::~QObject() () from /usr/lib64/libQt5Core.so.5
+	 * #5  0x00007f0a8da7d27d in QQmlEngine::~QQmlEngine() () from /usr/lib64/libQt5Qml.so.5
+	 * #6  0x00007f0a8dafeb39 in QQmlApplicationEngine::~QQmlApplicationEngine() () from /usr/lib64/libQt5Qml.so.5
+	 * #7  0x00007f0a8d61359c in QObjectPrivate::deleteChildren() () from /usr/lib64/libQt5Core.so.5
+	 * #8  0x00007f0a8d61ab83 in QObject::~QObject() () from /usr/lib64/libQt5Core.so.5
+	 * #9  0x00007f0a8d5ede58 in QCoreApplication::~QCoreApplication() () from /usr/lib64/libQt5Core.so.5
+	 * #10 0x00007f0a8dd2d97b in QGuiApplication::~QGuiApplication() () from /usr/lib64/libQt5Gui.so.5
+	 * #11 0x000000000041b6b4 in main (argc=1, argv=0x7fff218dd1a8) at ../../../../Development/rs-develop/retroshare-qml-app/src/main-app.cpp:58
+	 *
+	 * To avoid this we leave the creation of the instance to the user (main)
+	 * and to store the static pointer to that, the pointer can be null at early
+	 * stage of execution (or if the user forget to initialize it properly) so
+	 * early user (JNI intent handler) should take this in account
+	 */
+	static RsQmlAppEngine* mMainInstance;
+};