From ccaeb6539d2f7e9ac756a7f2017c9f9e8813e265 Mon Sep 17 00:00:00 2001 From: Alex Blasche Date: Wed, 26 Feb 2014 11:08:41 +0100 Subject: Fix crash when interrupting QBluetoothSocket's input stream thread The previous QThread did not always properly resume when InputStream.read() was interrupted by BluetoothSocket.close(). This patch converts the QThread to a Java thread which works as the Android API docs suggested. Task-number: QTBUG-37061 Change-Id: Id6ac9b57a28f3b532cbe49ff1dfdc9d1e6432aaa Reviewed-by: Nedim Hadzic Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/android/bluetooth/bluetooth.pri | 3 +- .../bluetooth/QtBluetoothInputStreamThread.java | 103 +++++++++++++++++++ src/bluetooth/android/inputstreamthread.cpp | 110 ++++++++------------- src/bluetooth/android/inputstreamthread_p.h | 21 ++-- src/bluetooth/android/jni_android.cpp | 28 ++++++ src/bluetooth/qbluetoothsocket_android.cpp | 87 ++++++++++++---- src/bluetooth/qbluetoothsocket_p.h | 2 +- 7 files changed, 253 insertions(+), 101 deletions(-) create mode 100644 src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothInputStreamThread.java diff --git a/src/android/bluetooth/bluetooth.pri b/src/android/bluetooth/bluetooth.pri index d3fdd0d0..b2121ff3 100644 --- a/src/android/bluetooth/bluetooth.pri +++ b/src/android/bluetooth/bluetooth.pri @@ -6,7 +6,8 @@ PATHPREFIX = $$PWD/src/org/qtproject/qt5/android/bluetooth JAVACLASSPATH += $$PWD/src/ JAVASOURCES += \ $$PATHPREFIX/QtBluetoothBroadcastReceiver.java \ - $$PATHPREFIX/QtBluetoothSocketServer.java + $$PATHPREFIX/QtBluetoothSocketServer.java \ + $$PATHPREFIX/QtBluetoothInputStreamThread.java # install diff --git a/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothInputStreamThread.java b/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothInputStreamThread.java new file mode 100644 index 00000000..57636ad4 --- /dev/null +++ b/src/android/bluetooth/src/org/qtproject/qt5/android/bluetooth/QtBluetoothInputStreamThread.java @@ -0,0 +1,103 @@ +/**************************************************************************** +** +** Copyright (C) 2014 Digia Plc and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/legal +** +** This file is part of the QtBluetooth module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and Digia. For licensing terms and +** conditions see http://qt.digia.com/licensing. For further information +** use the contact form at http://qt.digia.com/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 2.1 requirements +** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Digia gives you certain additional +** rights. These rights are described in the Digia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3.0 as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL included in the +** packaging of this file. Please review the following information to +** ensure the GNU General Public License version 3.0 requirements will be +** met: http://www.gnu.org/copyleft/gpl.html. +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +package org.qtproject.qt5.android.bluetooth; + +import java.io.InputStream; +import java.io.IOException; +import android.util.Log; + +public class QtBluetoothInputStreamThread extends Thread +{ + /* Pointer to the Qt object that "owns" the Java object */ + long qtObject = 0; + public boolean logEnabled = false; + private static final String TAG = "QtBluetooth"; + private InputStream m_inputStream = null; + + //error codes + public static final int QT_MISSING_INPUT_STREAM = 0; + public static final int QT_READ_FAILED = 1; + public static final int QT_THREAD_INTERRUPTED = 2; + + public QtBluetoothInputStreamThread() + { + setName("QtBtInputStreamThread"); + } + + public void setInputStream(InputStream stream) + { + m_inputStream = stream; + } + + public void run() + { + if (m_inputStream == null) { + errorOccurred(qtObject, QT_MISSING_INPUT_STREAM); + return; + } + + byte[] buffer = new byte[1000]; + int bytesRead = 0; + + try { + while (!isInterrupted()) { + //this blocks until we see incoming data + //or close() on related BluetoothSocket is called + bytesRead = m_inputStream.read(buffer); + readyData(qtObject, buffer, bytesRead); + } + + errorOccurred(qtObject, QT_THREAD_INTERRUPTED); + } catch (IOException ex) { + if (logEnabled) + Log.d(TAG, "InputStream.read() failed:" + ex.toString()); + ex.printStackTrace(); + errorOccurred(qtObject, QT_READ_FAILED); + } + + if (logEnabled) + Log.d(TAG, "Leaving input stream thread"); + } + + public static native void errorOccurred(long qtObject, int errorCode); + public static native void readyData(long qtObject, byte[] buffer, int bufferLength); +} diff --git a/src/bluetooth/android/inputstreamthread.cpp b/src/bluetooth/android/inputstreamthread.cpp index df32ee62..7f5029d9 100644 --- a/src/bluetooth/android/inputstreamthread.cpp +++ b/src/bluetooth/android/inputstreamthread.cpp @@ -40,6 +40,7 @@ ** ****************************************************************************/ +#include #include #include "android/inputstreamthread_p.h" @@ -47,34 +48,29 @@ QT_BEGIN_NAMESPACE +Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID) InputStreamThread::InputStreamThread(QBluetoothSocketPrivate *socket) - : QThread(), m_stop(false) + : QObject(), m_socket_p(socket), expectClosure(false) { - m_socket_p = socket; } -void InputStreamThread::run() +bool InputStreamThread::run() { - qint32 byte; - Q_UNUSED(byte) - while (1) { - { - QMutexLocker locker(&m_mutex); - if (m_stop) - break; - } - readFromInputStream(); - } + QMutexLocker lock(&m_mutex); - QAndroidJniEnvironment env; - if (m_socket_p->inputStream.isValid()) - m_socket_p->inputStream.callMethod("close"); + javaInputStreamThread = QAndroidJniObject("org/qtproject/qt5/android/bluetooth/QtBluetoothInputStreamThread"); + if (!javaInputStreamThread.isValid() || !m_socket_p->inputStream.isValid()) + return false; + + javaInputStreamThread.callMethod("setInputStream", "(Ljava/io/InputStream;)V", + m_socket_p->inputStream.object()); + javaInputStreamThread.setField("qtObject", reinterpret_cast(this)); + javaInputStreamThread.setField("logEnabled", QT_BT_ANDROID().isDebugEnabled()); + + javaInputStreamThread.callMethod("start"); - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); - } + return true; } bool InputStreamThread::bytesAvailable() const @@ -83,68 +79,42 @@ bool InputStreamThread::bytesAvailable() const return m_socket_p->buffer.size(); } -//This runs inside the thread. -void InputStreamThread::readFromInputStream() +qint64 InputStreamThread::readData(char *data, qint64 maxSize) { - QAndroidJniEnvironment env; - - int bufLen = 1000; // Seems to magical number that also low-end products can survive. - jbyteArray nativeArray = env->NewByteArray(bufLen); - + QMutexLocker locker(&m_mutex); - jint ret = m_socket_p->inputStream.callMethod("read", "([BII)I", nativeArray, 0, bufLen); + if (!m_socket_p->buffer.isEmpty()) + return m_socket_p->buffer.read(data, maxSize); - if (env->ExceptionCheck() || ret < 0) { - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); - } - env->DeleteLocalRef(nativeArray); - QMutexLocker lock(&m_mutex); - m_stop = true; + return 0; +} - /* - * We cannot distinguish IOException due to valid closure or due to other error - * Therefore we always have to throw an error and a disconnect signal - * A genuine disconnect wouldn't need the error signal. - * For now we always signal error which implicitly emits disconnect() too. - */ +//inside the java thread +void InputStreamThread::javaThreadErrorOccurred(int errorCode) +{ + QMutexLocker lock(&m_mutex); - emit error(); - return; - } + if (!expectClosure) + emit error(errorCode); + else + emit error(-1); //magic error, -1 means error was expected due to expected close() +} - if (ret == 0) { - qDebug() << "Nothing to read"; - env->DeleteLocalRef(nativeArray); - return; - } +//inside the java thread +void InputStreamThread::javaReadyRead(jbyteArray buffer, int bufferLength) +{ + QAndroidJniEnvironment env; QMutexLocker lock(&m_mutex); - char *writePtr = m_socket_p->buffer.reserve(bufLen); - env->GetByteArrayRegion(nativeArray, 0, ret, reinterpret_cast(writePtr)); - env->DeleteLocalRef(nativeArray); - m_socket_p->buffer.chop(bufLen - ret); + char *writePtr = m_socket_p->buffer.reserve(bufferLength); + env->GetByteArrayRegion(buffer, 0, bufferLength, reinterpret_cast(writePtr)); emit dataAvailable(); } -void InputStreamThread::stop() -{ - QMutexLocker locker(&m_mutex); - m_stop = true; -} - -qint64 InputStreamThread::readData(char *data, qint64 maxSize) +void InputStreamThread::prepareForClosure() { - QMutexLocker locker(&m_mutex); - - if (m_stop) - return -1; - - if (!m_socket_p->buffer.isEmpty()) - return m_socket_p->buffer.read(data, maxSize); - - return 0; + QMutexLocker lock(&m_mutex); + expectClosure = true; } QT_END_NAMESPACE diff --git a/src/bluetooth/android/inputstreamthread_p.h b/src/bluetooth/android/inputstreamthread_p.h index 85852534..8b565cff 100644 --- a/src/bluetooth/android/inputstreamthread_p.h +++ b/src/bluetooth/android/inputstreamthread_p.h @@ -43,34 +43,39 @@ #ifndef INPUTSTREAMTHREAD_H #define INPUTSTREAMTHREAD_H -#include +#include #include +#include +#include QT_BEGIN_NAMESPACE class QBluetoothSocketPrivate; -class InputStreamThread : public QThread +class InputStreamThread : public QObject { Q_OBJECT public: explicit InputStreamThread(QBluetoothSocketPrivate *socket_p); - virtual void run(); bool bytesAvailable() const; - void stop(); + bool run(); qint64 readData(char *data, qint64 maxSize); + void javaThreadErrorOccurred(int errorCode); + void javaReadyRead(jbyteArray buffer, int bufferLength); + + void prepareForClosure(); + signals: void dataAvailable(); - void error(); + void error(int errorCode); private: - void readFromInputStream(); - QBluetoothSocketPrivate *m_socket_p; + QAndroidJniObject javaInputStreamThread; mutable QMutex m_mutex; - bool m_stop; + bool expectClosure; }; QT_END_NAMESPACE diff --git a/src/bluetooth/android/jni_android.cpp b/src/bluetooth/android/jni_android.cpp index ce0f19ca..1e907fdd 100644 --- a/src/bluetooth/android/jni_android.cpp +++ b/src/bluetooth/android/jni_android.cpp @@ -47,6 +47,7 @@ #include #include "android/androidbroadcastreceiver_p.h" #include "android/serveracceptancethread_p.h" +#include "android/inputstreamthread_p.h" Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID) @@ -68,6 +69,19 @@ static void QtBluetoothSocketServer_newSocket(JNIEnv */*env*/, jobject /*javaObj reinterpret_cast(qtObject)->javaNewSocket(socket); } +static void QtBluetoothInputStreamThread_errorOccurred(JNIEnv */*env*/, jobject /*javaObject*/, + jlong qtObject, jint errorCode) +{ + reinterpret_cast(qtObject)->javaThreadErrorOccurred(errorCode); +} + +static void QtBluetoothInputStreamThread_readyData(JNIEnv */*env*/, jobject /*javaObject*/, + jlong qtObject, jbyteArray buffer, jint bufferLength) +{ + reinterpret_cast(qtObject)->javaReadyRead(buffer, bufferLength); +} + + static JNINativeMethod methods[] = { {"jniOnReceive", "(JLandroid/content/Context;Landroid/content/Intent;)V", (void *) QtBroadcastReceiver_jniOnReceive}, @@ -80,6 +94,13 @@ static JNINativeMethod methods_server[] = { (void *) QtBluetoothSocketServer_newSocket}, }; +static JNINativeMethod methods_inputStream[] = { + {"errorOccurred", "(JI)V", + (void *) QtBluetoothInputStreamThread_errorOccurred}, + {"readyData", "(J[BI)V", + (void *) QtBluetoothInputStreamThread_readyData}, +}; + static const char logTag[] = "QtBluetooth"; static const char classErrorMsg[] = "Can't find class \"%s\""; @@ -107,6 +128,13 @@ static bool registerNatives(JNIEnv *env) return false; } + FIND_AND_CHECK_CLASS("org/qtproject/qt5/android/bluetooth/QtBluetoothInputStreamThread"); + if (env->RegisterNatives(clazz, methods_inputStream, + sizeof(methods_inputStream) / sizeof(methods_inputStream[0])) < 0) { + __android_log_print(ANDROID_LOG_FATAL, logTag, "RegisterNatives for InputStreamThread failed"); + return false; + } + return true; } diff --git a/src/bluetooth/qbluetoothsocket_android.cpp b/src/bluetooth/qbluetoothsocket_android.cpp index 9f04c69e..5c84e627 100644 --- a/src/bluetooth/qbluetoothsocket_android.cpp +++ b/src/bluetooth/qbluetoothsocket_android.cpp @@ -170,9 +170,7 @@ void QBluetoothSocketPrivate::connectToServiceConc(const QBluetoothAddress &addr } if (inputThread) { - inputThread->stop(); - inputThread->wait(); - delete inputThread; + inputThread->deleteLater(); inputThread = 0; } @@ -200,10 +198,29 @@ void QBluetoothSocketPrivate::connectToServiceConc(const QBluetoothAddress &addr } inputThread = new InputStreamThread(this); - QObject::connect(inputThread, SIGNAL(dataAvailable()), q, SIGNAL(readyRead()), Qt::QueuedConnection); + QObject::connect(inputThread, SIGNAL(dataAvailable()), + q, SIGNAL(readyRead()), Qt::QueuedConnection); QObject::connect(inputThread, SIGNAL(error()), this, SLOT(inputThreadError()), Qt::QueuedConnection); - inputThread->start(); + + if (!inputThread->run()) { + //close socket again + socketObject.callMethod("close"); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); + } + + socketObject = inputStream = outputStream = remoteDevice = QAndroidJniObject(); + + delete inputThread; + inputThread = 0; + + errorString = QBluetoothSocket::tr("Input stream thread cannot be started"); + q->setSocketError(QBluetoothSocket::NetworkError); + q->setSocketState(QBluetoothSocket::UnconnectedState); + return; + } q->setSocketState(QBluetoothSocket::ConnectedState); emit q->connected(); @@ -232,6 +249,10 @@ void QBluetoothSocketPrivate::abort() * new state, error and emits relevant signals. * See QBluetoothSocketPrivate::inputThreadError() for details */ + + if (inputThread) + inputThread->prepareForClosure(); + //triggers abort of input thread as well socketObject.callMethod("close"); if (env->ExceptionCheck()) { @@ -242,9 +263,9 @@ void QBluetoothSocketPrivate::abort() } if (inputThread) { - inputThread->stop(); - inputThread->wait(); - delete inputThread; + //don't delete here as signals caused by Java Thread are still + //going to be emitted + //delete occurs in inputThreadError() inputThread = 0; } @@ -306,7 +327,7 @@ qint64 QBluetoothSocketPrivate::writeData(const char *data, qint64 maxSize) //TODO check that readData and writeData return -1 on error (on all platforms) Q_Q(QBluetoothSocket); if (state != QBluetoothSocket::ConnectedState || !outputStream.isValid()) { - qCWarning(QT_BT_ANDROID) << "Socket::writeData: " << (int)state << outputStream.isValid() ; + qCWarning(QT_BT_ANDROID) << "Socket::writeData: " << state << outputStream.isValid(); errorString = QBluetoothSocket::tr("Cannot write while not connected"); q->setSocketError(QBluetoothSocket::OperationError); return -1; @@ -334,7 +355,7 @@ qint64 QBluetoothSocketPrivate::readData(char *data, qint64 maxSize) { Q_Q(QBluetoothSocket); if (state != QBluetoothSocket::ConnectedState || !inputThread) { - qCWarning(QT_BT_ANDROID) << "Socket::writeData: " << (int)state << outputStream.isValid() ; + qCWarning(QT_BT_ANDROID) << "Socket::readData: " << state << inputThread ; errorString = QBluetoothSocket::tr("Cannot write while not connected"); q->setSocketError(QBluetoothSocket::OperationError); return -1; @@ -343,13 +364,38 @@ qint64 QBluetoothSocketPrivate::readData(char *data, qint64 maxSize) return inputThread->readData(data, maxSize); } -void QBluetoothSocketPrivate::inputThreadError() +void QBluetoothSocketPrivate::inputThreadError(int errorCode) { Q_Q(QBluetoothSocket); - //any error from InputThread is a NetworkError - errorString = QBluetoothSocket::tr("Network error during read"); - q->setSocketError(QBluetoothSocket::NetworkError); + if (errorCode != -1) { //magic error which is expected and can be ignored + errorString = QBluetoothSocket::tr("Network error during read"); + q->setSocketError(QBluetoothSocket::NetworkError); + } + + //finally we can delete the InputStreamThread + InputStreamThread *client = qobject_cast(sender()); + if (client) + client->deleteLater(); + + if (socketObject.isValid()) { + //triggered when remote side closed the socket + //cleanup internal objects + //if it was call to local close()/abort() the objects are cleaned up already + + bool stillConnected = socketObject.callMethod("isConnected"); + if (stillConnected) { + QAndroidJniEnvironment env; + socketObject.callMethod("close"); + if (env->ExceptionCheck()) { + env->ExceptionDescribe(); + env->ExceptionClear(); + } + } + + inputStream = outputStream = remoteDevice = socketObject = QAndroidJniObject(); + } + q->setSocketState(QBluetoothSocket::UnconnectedState); emit q->disconnected(); } @@ -414,16 +460,15 @@ bool QBluetoothSocketPrivate::setSocketDescriptor(const QAndroidJniObject &socke remoteDevice = socketObject.callObjectMethod("getRemoteDevice", "()Landroid/bluetooth/BluetoothDevice;"); if (inputThread) { - inputThread->stop(); - inputThread->wait(); - delete inputThread; + inputThread->deleteLater(); inputThread = 0; } inputThread = new InputStreamThread(this); - QObject::connect(inputThread, SIGNAL(dataAvailable()), q, SIGNAL(readyRead()), Qt::QueuedConnection); - QObject::connect(inputThread, SIGNAL(error()), - this, SLOT(inputThreadError()), Qt::QueuedConnection); - inputThread->start(); + QObject::connect(inputThread, SIGNAL(dataAvailable()), + q, SIGNAL(readyRead()), Qt::QueuedConnection); + QObject::connect(inputThread, SIGNAL(error(int)), + this, SLOT(inputThreadError(int)), Qt::QueuedConnection); + inputThread->run(); q->setSocketState(socketState); diff --git a/src/bluetooth/qbluetoothsocket_p.h b/src/bluetooth/qbluetoothsocket_p.h index 2e1bb66f..2fabeba1 100644 --- a/src/bluetooth/qbluetoothsocket_p.h +++ b/src/bluetooth/qbluetoothsocket_p.h @@ -170,7 +170,7 @@ public: InputStreamThread *inputThread; private Q_SLOTS: - void inputThreadError(); + void inputThreadError(int errorCode); #endif -- cgit v1.2.3