From b93dc2756d9947236deb955634b499b27aaaa414 Mon Sep 17 00:00:00 2001 From: Alex Blasche Date: Wed, 28 Jun 2017 16:21:01 +0200 Subject: Fix concurrent access to QBluetoothSocket::canReadLine() on Android On Android, the socket's read buffer is managed in the Java thread. QBluetoothSocket::canReadLine() is public API (most likely) being called by in the main Qt thread though. The function directly called into the Java buffer instance without proper locking. This can create race conditions. Starting with this patch canReadLine() calls another QBluetoothSocketPrivate indirection to allow a platform specific implementation. This affects WinRT, BlueZ and the dummy backend too. This is not an issue on macOS as its implementation of QBluetoothSocket is separate and does not have to deal with multiple QBluetoothSocketPrivate implementations. Task-number: QTBUG-58190 Task-number: QTBUG-60830 Change-Id: Idae19f1aee6f809699d36519b01a3c68ad9c563d Reviewed-by: Qt CI Bot Reviewed-by: Oliver Wolff --- src/bluetooth/android/inputstreamthread.cpp | 6 ++++++ src/bluetooth/android/inputstreamthread_p.h | 1 + src/bluetooth/qbluetoothsocket.cpp | 2 +- src/bluetooth/qbluetoothsocket_android.cpp | 9 +++++++++ src/bluetooth/qbluetoothsocket_bluez.cpp | 5 +++++ src/bluetooth/qbluetoothsocket_p.cpp | 5 +++++ src/bluetooth/qbluetoothsocket_p.h | 1 + src/bluetooth/qbluetoothsocket_winrt.cpp | 5 +++++ 8 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/bluetooth/android/inputstreamthread.cpp b/src/bluetooth/android/inputstreamthread.cpp index ecd9218e..982c477b 100644 --- a/src/bluetooth/android/inputstreamthread.cpp +++ b/src/bluetooth/android/inputstreamthread.cpp @@ -77,6 +77,12 @@ qint64 InputStreamThread::bytesAvailable() const return m_socket_p->buffer.size(); } +bool InputStreamThread::canReadLine() const +{ + QMutexLocker locker(&m_mutex); + return m_socket_p->buffer.canReadLine(); +} + qint64 InputStreamThread::readData(char *data, qint64 maxSize) { QMutexLocker locker(&m_mutex); diff --git a/src/bluetooth/android/inputstreamthread_p.h b/src/bluetooth/android/inputstreamthread_p.h index 741333ac..a6ee0655 100644 --- a/src/bluetooth/android/inputstreamthread_p.h +++ b/src/bluetooth/android/inputstreamthread_p.h @@ -68,6 +68,7 @@ public: explicit InputStreamThread(QBluetoothSocketPrivate *socket_p); qint64 bytesAvailable() const; + bool canReadLine() const; bool run(); qint64 readData(char *data, qint64 maxSize); diff --git a/src/bluetooth/qbluetoothsocket.cpp b/src/bluetooth/qbluetoothsocket.cpp index 3e961142..57dd3aa0 100644 --- a/src/bluetooth/qbluetoothsocket.cpp +++ b/src/bluetooth/qbluetoothsocket.cpp @@ -624,7 +624,7 @@ void QBluetoothSocket::setSocketState(QBluetoothSocket::SocketState state) bool QBluetoothSocket::canReadLine() const { Q_D(const QBluetoothSocket); - return d->buffer.canReadLine() || QIODevice::canReadLine(); + return d->canReadLine(); } /*! diff --git a/src/bluetooth/qbluetoothsocket_android.cpp b/src/bluetooth/qbluetoothsocket_android.cpp index 87c9ffc8..c2fc96f2 100644 --- a/src/bluetooth/qbluetoothsocket_android.cpp +++ b/src/bluetooth/qbluetoothsocket_android.cpp @@ -879,6 +879,15 @@ qint64 QBluetoothSocketPrivate::bytesAvailable() const return 0; } +bool QBluetoothSocketPrivate::canReadLine() const +{ + // We cannot access buffer directly as it is part of different thread + if (inputThread) + return inputThread->canReadLine(); + + return false; +} + QT_END_NAMESPACE #include diff --git a/src/bluetooth/qbluetoothsocket_bluez.cpp b/src/bluetooth/qbluetoothsocket_bluez.cpp index 42c5503b..dca1bc3b 100644 --- a/src/bluetooth/qbluetoothsocket_bluez.cpp +++ b/src/bluetooth/qbluetoothsocket_bluez.cpp @@ -592,4 +592,9 @@ qint64 QBluetoothSocketPrivate::bytesAvailable() const return buffer.size(); } +bool QBluetoothSocketPrivate::canReadLine() const +{ + return buffer.canReadLine(); +} + QT_END_NAMESPACE diff --git a/src/bluetooth/qbluetoothsocket_p.cpp b/src/bluetooth/qbluetoothsocket_p.cpp index 6007b924..dc14e97b 100644 --- a/src/bluetooth/qbluetoothsocket_p.cpp +++ b/src/bluetooth/qbluetoothsocket_p.cpp @@ -158,4 +158,9 @@ qint64 QBluetoothSocketPrivate::bytesAvailable() const return 0; } +bool QBluetoothSocketPrivate::canReadLine() const +{ + return false; +} + QT_END_NAMESPACE diff --git a/src/bluetooth/qbluetoothsocket_p.h b/src/bluetooth/qbluetoothsocket_p.h index 177f154a..cfa342bb 100644 --- a/src/bluetooth/qbluetoothsocket_p.h +++ b/src/bluetooth/qbluetoothsocket_p.h @@ -166,6 +166,7 @@ public: QBluetoothSocket::OpenMode openMode = QBluetoothSocket::ReadWrite); qint64 bytesAvailable() const; + bool canReadLine() const; public: QPrivateLinearBuffer buffer; diff --git a/src/bluetooth/qbluetoothsocket_winrt.cpp b/src/bluetooth/qbluetoothsocket_winrt.cpp index 1f4e6233..a56e10fa 100644 --- a/src/bluetooth/qbluetoothsocket_winrt.cpp +++ b/src/bluetooth/qbluetoothsocket_winrt.cpp @@ -569,6 +569,11 @@ qint64 QBluetoothSocketPrivate::bytesAvailable() const return buffer.size(); } +bool QBluetoothSocketPrivate::canReadLine() const +{ + return buffer.canReadLine(); +} + void QBluetoothSocketPrivate::handleNewData(const QVector &data) { // Defer putting the data into the list until the next event loop iteration -- cgit v1.2.3