Android: Fix crash in QBluetoothServer::close()

Java's BluetoothSocketServer.accept() is meant to be interrupted
via BluetoothSocketServer.close(). Unfortunately if the surrounding
thread is a QThread the returning accept call crashes the thread. This
does not happen if it is a Java Thread.

This commit changes the server's private backend to a Java thread.

Task-number: QTBUG-36754
Change-Id: I5aacc5444bbcd1275a11743b6aa04d2b11a5b22b
Reviewed-by: Nedim Hadzic <nedimhadzija@gmail.com>
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@digia.com>

3 files changed, 122 insertions, 137 deletions

diff --git a/src/bluetooth/android/jni_android.cpp b/src/bluetooth/android/jni_android.cpp
index eb1fc2dd..ce0f19ca 100644
--- a/src/bluetooth/android/jni_android.cpp
+++ b/src/bluetooth/android/jni_android.cpp
@@ -46,6 +46,7 @@
 #include <QtBluetooth/qbluetoothglobal.h>
 #include <QtAndroidExtras/QAndroidJniObject>
 #include "android/androidbroadcastreceiver_p.h"
+#include "android/serveracceptancethread_p.h"
 
 Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID)
 
@@ -55,9 +56,28 @@ void QtBroadcastReceiver_jniOnReceive(JNIEnv *env, jobject /*javaObject*/,
     reinterpret_cast<AndroidBroadcastReceiver*>(qtObject)->onReceive(env, context, intent);
 }
 
+static void QtBluetoothSocketServer_errorOccurred(JNIEnv */*env*/, jobject /*javaObject*/,
+                                           jlong qtObject, jint errorCode)
+{
+    reinterpret_cast<ServerAcceptanceThread*>(qtObject)->javaThreadErrorOccurred(errorCode);
+}
+
+static void QtBluetoothSocketServer_newSocket(JNIEnv */*env*/, jobject /*javaObject*/,
+                                       jlong qtObject, jobject socket)
+{
+    reinterpret_cast<ServerAcceptanceThread*>(qtObject)->javaNewSocket(socket);
+}
+
 static JNINativeMethod methods[] = {
     {"jniOnReceive", "(JLandroid/content/Context;Landroid/content/Intent;)V",
-               (void *) QtBroadcastReceiver_jniOnReceive},
+                (void *) QtBroadcastReceiver_jniOnReceive},
+};
+
+static JNINativeMethod methods_server[] = {
+        {"errorOccurred", "(JI)V",
+                    (void *) QtBluetoothSocketServer_errorOccurred},
+        {"newSocket", "(JLandroid/bluetooth/BluetoothSocket;)V",
+                    (void *) QtBluetoothSocketServer_newSocket},
 };
 
 static const char logTag[] = "QtBluetooth";
@@ -75,8 +95,15 @@ static bool registerNatives(JNIEnv *env)
     jclass clazz;
     FIND_AND_CHECK_CLASS("org/qtproject/qt5/android/bluetooth/QtBluetoothBroadcastReceiver");
 
+
     if (env->RegisterNatives(clazz, methods, sizeof(methods) / sizeof(methods[0])) < 0) {
-        __android_log_print(ANDROID_LOG_FATAL, logTag, "RegisterNatives failed");
+        __android_log_print(ANDROID_LOG_FATAL, logTag, "RegisterNatives for BraodcastReceiver failed");
+        return false;
+    }
+
+    FIND_AND_CHECK_CLASS("org/qtproject/qt5/android/bluetooth/QtBluetoothSocketServer");
+    if (env->RegisterNatives(clazz, methods_server, sizeof(methods_server) / sizeof(methods_server[0])) < 0) {
+        __android_log_print(ANDROID_LOG_FATAL, logTag, "RegisterNatives for SocketServer failed");
         return false;
     }
 
diff --git a/src/bluetooth/android/serveracceptancethread.cpp b/src/bluetooth/android/serveracceptancethread.cpp
index 88a92478..d46fff7c 100644
--- a/src/bluetooth/android/serveracceptancethread.cpp
+++ b/src/bluetooth/android/serveracceptancethread.cpp
@@ -47,11 +47,9 @@
 Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID)
 
 ServerAcceptanceThread::ServerAcceptanceThread(QObject *parent) :
-    QThread(parent), m_stop(false), maxPendingConnections(1)
+    QObject(parent), maxPendingConnections(1)
 {
-    btAdapter = QAndroidJniObject::callStaticObjectMethod("android/bluetooth/BluetoothAdapter",
-                                                          "getDefaultAdapter",
-                                                          "()Landroid/bluetooth/BluetoothAdapter;");
+    qRegisterMetaType<QBluetoothServer::Error>("QBluetoothServer::Error");
 }
 
 ServerAcceptanceThread::~ServerAcceptanceThread()
@@ -71,161 +69,121 @@ void ServerAcceptanceThread::setServiceDetails(const QBluetoothUuid &uuid,
     secFlags = securityFlags;
 }
 
+bool ServerAcceptanceThread::hasPendingConnections() const
+{
+    QMutexLocker lock(&m_mutex);
+    return (pendingSockets.count() > 0);
+}
+
+/*
+ * Returns the next pending connection or an invalid JNI object.
+ * Note that even a stopped thread may still have pending
+ * connections. Pending connections are only terminated upon
+ * thread restart or destruction.
+ */
+QAndroidJniObject ServerAcceptanceThread::nextPendingConnection()
+{
+    QMutexLocker lock(&m_mutex);
+    if (pendingSockets.isEmpty())
+        return QAndroidJniObject();
+    else
+        return pendingSockets.takeFirst();
+}
+
+void ServerAcceptanceThread::setMaxPendingConnections(int maximumCount)
+{
+    QMutexLocker lock(&m_mutex);
+    maxPendingConnections = maximumCount;
+}
+
 void ServerAcceptanceThread::run()
 {
-    m_mutex.lock();
+    QMutexLocker lock(&m_mutex);
 
-    qCDebug(QT_BT_ANDROID) << "Starting ServerSocketAccept thread";
     if (!validSetup()) {
         qCWarning(QT_BT_ANDROID) << "Invalid Server Socket setup";
-        m_mutex.unlock();
         return;
     }
 
-    shutdownPendingConnections();
-
-    m_stop = false;
-
-    QString tempUuid = m_uuid.toString();
-    tempUuid.chop(1); //remove trailing '}'
-    tempUuid.remove(0,1); //remove first '{'
-
-    QAndroidJniEnvironment env;
-    QAndroidJniObject inputString = QAndroidJniObject::fromString(tempUuid);
-    QAndroidJniObject uuidObject = QAndroidJniObject::callStaticObjectMethod(
-                "java/util/UUID", "fromString",
-                "(Ljava/lang/String;)Ljava/util/UUID;",
-                inputString.object<jstring>());
-    inputString = QAndroidJniObject::fromString(m_serviceName);
-    if (((int)secFlags) == 0) { //no form of security flag set
-        qCDebug(QT_BT_ANDROID) << "InSecure listening";
-        btServerSocket = btAdapter.callObjectMethod("listenUsingInsecureRfcommWithServiceRecord",
-                                                "(Ljava/lang/String;Ljava/util/UUID;)Landroid/bluetooth/BluetoothServerSocket;",
-                                                inputString.object<jstring>(),
-                                                uuidObject.object<jobject>());
-    } else {
-        qCDebug(QT_BT_ANDROID) << "Secure listening";
-        btServerSocket = btAdapter.callObjectMethod("listenUsingRfcommWithServiceRecord",
-                                                    "(Ljava/lang/String;Ljava/util/UUID;)Landroid/bluetooth/BluetoothServerSocket;",
-                                                    inputString.object<jstring>(),
-                                                    uuidObject.object<jobject>());
-    }
-    if (env->ExceptionCheck()) {
-        env->ExceptionDescribe();
-        env->ExceptionClear();
-        qCWarning(QT_BT_ANDROID) << "Cannot setup rfcomm socket listener";
-        m_mutex.unlock();
-        return;
+    if (isRunning()) {
+        stop();
+        shutdownPendingConnections();
     }
 
-    if (!btServerSocket.isValid()) {
-        qCWarning(QT_BT_ANDROID) << "Invalid BluetoothServerSocket";
-        m_mutex.unlock();
+    javaThread = QAndroidJniObject("org/qtproject/qt5/android/bluetooth/QtBluetoothSocketServer");
+    if (!javaThread.isValid())
         return;
-    }
-
-    while (!m_stop) {
-        m_mutex.unlock();
-
-        qCDebug(QT_BT_ANDROID) << "Waiting for new incoming socket";
-        QAndroidJniEnvironment env;
-
-        //this call blocks until we see an incoming connection
-        QAndroidJniObject socket = btServerSocket.callObjectMethod("accept",
-                                                                   "()Landroid/bluetooth/BluetoothSocket;");
 
-        qCDebug(QT_BT_ANDROID) << "New socket accepted: ->" << socket.isValid();
-        bool exceptionOccurred = false;
-        if (env->ExceptionCheck()) {
-            env->ExceptionDescribe();
-            env->ExceptionClear();
-            exceptionOccurred = true;
-        }
+    javaThread.setField<jlong>("qtObject", reinterpret_cast<long>(this));
+    javaThread.setField<jboolean>("logEnabled", QT_BT_ANDROID().isDebugEnabled());
 
-        m_mutex.lock();
-
-        if (exceptionOccurred || m_stop) {
-            //if m_stop is true there is nothing really to be done but exit
-            m_stop = true;
-        } else if (socket.isValid()){
-            if (pendingSockets.count() < maxPendingConnections) {
-                pendingSockets.append(socket);
-                emit newConnection();
-            } else {
-                qCWarning(QT_BT_ANDROID) << "Refusing connection due to limited pending socket queue";
-                socket.callMethod<void>("close");
-                if (env->ExceptionCheck()) {
-                    qCWarning(QT_BT_ANDROID) << "Error during refusal of new socket";
-                    env->ExceptionDescribe();
-                    env->ExceptionClear();
-                }
-
-
-            }
-        } else {
-            //should never happen as invalid socket should cause exception
-            qCWarning(QT_BT_ANDROID) << "Invalid state during server socket accept";
-        }
-    }
+    QString tempUuid = m_uuid.toString();
+    tempUuid.chop(1); //remove trailing '}'
+    tempUuid.remove(0,1); //remove first '{'
 
-    m_uuid = QBluetoothUuid();
-    m_serviceName = QString();
-    btServerSocket = QAndroidJniObject();
-    m_mutex.unlock();
+    QAndroidJniObject uuidString = QAndroidJniObject::fromString(tempUuid);
+    QAndroidJniObject serviceNameString = QAndroidJniObject::fromString(m_serviceName);
+    bool isSecure = !(secFlags == QBluetooth::NoSecurity);
+    javaThread.callMethod<void>("setServiceDetails", "(Ljava/lang/String;Ljava/lang/String;Z)V",
+                                uuidString.object<jstring>(),
+                                serviceNameString.object<jstring>(),
+                                isSecure);
+    javaThread.callMethod<void>("start");
 }
 
 void ServerAcceptanceThread::stop()
 {
-    QMutexLocker lock(&m_mutex);
-    m_stop = true;
-
-    QAndroidJniEnvironment env;
-    if (btServerSocket.isValid()) {
+    if (javaThread.isValid()) {
         qCDebug(QT_BT_ANDROID) << "Closing server socket";
-        btServerSocket.callMethod<void>("close");
-        if (env->ExceptionCheck()) {
-            qCWarning(QT_BT_ANDROID) << "Exception during closure of server socket";
-            env->ExceptionDescribe();
-            env->ExceptionClear();
-        }
-        qCDebug(QT_BT_ANDROID) << "Closing server socket111";
+        javaThread.callMethod<void>("close");
     }
 }
 
-bool ServerAcceptanceThread::hasPendingConnections() const
+bool ServerAcceptanceThread::isRunning() const
 {
-    QMutexLocker lock(&m_mutex);
-    return (pendingSockets.count() > 0);
+    if (javaThread.isValid())
+        return javaThread.callMethod<jboolean>("isAlive");
+
+    return false;
 }
 
-/*
- * Returns the next pending connection or an invalid JNI object.
- * Note that even a stopped thread may still have pending
- * connections. Pending connections are only terminated upon
- * thread restart or destruction.
- */
-QAndroidJniObject ServerAcceptanceThread::nextPendingConnection()
+//Runs inside the java thread
+void ServerAcceptanceThread::javaThreadErrorOccurred(int errorCode)
 {
-    QMutexLocker lock(&m_mutex);
-    if (pendingSockets.isEmpty())
-        return QAndroidJniObject();
-    else
-        return pendingSockets.takeFirst();
+    qCDebug(QT_BT_ANDROID) << "JavaThread error:" << errorCode;
+    emit error(QBluetoothServer::InputOutputError);
 }
 
-void ServerAcceptanceThread::setMaxPendingConnections(int maximumCount)
+//Runs inside the Java thread
+void ServerAcceptanceThread::javaNewSocket(jobject s)
 {
     QMutexLocker lock(&m_mutex);
-    maxPendingConnections = maximumCount;
+
+    QAndroidJniObject socket(s);
+    if (!socket.isValid())
+       return;
+
+    if (pendingSockets.count() < maxPendingConnections) {
+        qCDebug(QT_BT_ANDROID) << "New incoming java socket detected";
+        pendingSockets.append(socket);
+        emit newConnection();
+    } else {
+        QAndroidJniEnvironment env;
+        qCWarning(QT_BT_ANDROID) << "Refusing connection due to limited pending socket queue";
+        socket.callMethod<void>("close");
+        if (env->ExceptionCheck()) {
+            qCWarning(QT_BT_ANDROID) << "Error during refusal of new socket";
+            env->ExceptionDescribe();
+            env->ExceptionClear();
+        }
+    }
 }
 
-//must be run inside the lock but doesn't lock by itself
 bool ServerAcceptanceThread::validSetup() const
 {
     return (!m_uuid.isNull() && !m_serviceName.isEmpty());
 }
 
-//must be run inside the lock but doesn't lock by itself
 void ServerAcceptanceThread::shutdownPendingConnections()
 {
     while (!pendingSockets.isEmpty()) {
diff --git a/src/bluetooth/android/serveracceptancethread_p.h b/src/bluetooth/android/serveracceptancethread_p.h
index 1297e48f..18142d64 100644
--- a/src/bluetooth/android/serveracceptancethread_p.h
+++ b/src/bluetooth/android/serveracceptancethread_p.h
@@ -43,50 +43,50 @@
 #define SERVERACCEPTANCETHREAD_H
 
 #include <QtCore/QMutex>
-#include <QtCore/QThread>
 #include <QtAndroidExtras/QAndroidJniObject>
+#include <QtBluetooth/QBluetoothServer>
 #include <QtBluetooth/QBluetoothUuid>
 #include "qbluetooth.h"
 
 
-class ServerAcceptanceThread : public QThread
+class ServerAcceptanceThread : public QObject
 {
     Q_OBJECT
 public:
-    enum AndroidError {
-        AndroidNoError
-    };
-
     explicit ServerAcceptanceThread(QObject *parent = 0);
     ~ServerAcceptanceThread();
     void setServiceDetails(const QBluetoothUuid &uuid, const QString &serviceName,
                            QBluetooth::SecurityFlags securityFlags);
-    virtual void run();
 
-    void stop();
+
     bool hasPendingConnections() const;
     QAndroidJniObject nextPendingConnection();
     void setMaxPendingConnections(int maximumCount);
 
+    void javaThreadErrorOccurred(int errorCode);
+    void javaNewSocket(jobject socket);
+
+    void run();
+    void stop();
+    bool isRunning() const;
+
 signals:
     void newConnection();
-    void error(ServerAcceptanceThread::AndroidError);
+    void error(QBluetoothServer::Error);
 
 private:
     bool validSetup() const;
     void shutdownPendingConnections();
 
     QList<QAndroidJniObject> pendingSockets;
-    QAndroidJniObject btAdapter;
-    QAndroidJniObject btServerSocket;
     mutable QMutex m_mutex;
     QString m_serviceName;
     QBluetoothUuid m_uuid;
-    bool m_stop;
-    AndroidError lastError;
     int maxPendingConnections;
     QBluetooth::SecurityFlags secFlags;
 
+    QAndroidJniObject javaThread;
+
 };
 
 #endif // SERVERACCEPTANCETHREAD_H