summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDenis Shienkov <denis.shienkov@gmail.com>2014-08-14 21:50:05 +0400
committerDenis Shienkov <denis.shienkov@gmail.com>2014-08-28 21:17:50 +0200
commitb4d5bd813f591dc618e0fff2d55d93eeccb1a26e (patch)
tree1b08f7bea9b804501ce4b2181b1d66361a0059c8
parent286d64c202f461cb8ff5a4cc4c2320af8195b8f0 (diff)
Fix a race condition at writing on Windowsv5.3.2
1. The writeStarted flag shall be reset to false after bytesWritten() signal is emitted, but not before it. 2. In case of start of writing from the startAsyncWriteTimer it is necessary to call the startAsyncWrite() method but not the completeAsyncWrite() method. It give guarantees to exclude of race condition to multiple call of the WriteFile twice until the write operation is not completed. For example, a race condition is watched in case of writing data from the slots which are connected to the bytesWritten() signal from a device; or to the timeout() signal from a timer with zero interval. To checking of these scenarios are added additional autotests. Tested on Windows 8 with the on-board and USB serial ports, using Qt5. Task-number: QTBUG-40769 Change-Id: Iedabbf38847debeee795a10fd7a4c54c65d2a338 Reviewed-by: Patrick Noffke <patrick.noffke@gmail.com> Reviewed-by: Denis Shienkov <denis.shienkov@gmail.com>
-rw-r--r--src/serialport/qserialport.h1
-rw-r--r--src/serialport/qserialport_win.cpp19
-rw-r--r--src/serialport/qserialport_win_p.h2
-rw-r--r--tests/auto/qserialport/tst_qserialport.cpp155
4 files changed, 167 insertions, 10 deletions
diff --git a/src/serialport/qserialport.h b/src/serialport/qserialport.h
index d0f65b5e..8a0bbb11 100644
--- a/src/serialport/qserialport.h
+++ b/src/serialport/qserialport.h
@@ -287,6 +287,7 @@ private:
Q_PRIVATE_SLOT(d_func(), bool _q_completeAsyncCommunication())
Q_PRIVATE_SLOT(d_func(), bool _q_completeAsyncRead())
Q_PRIVATE_SLOT(d_func(), bool _q_completeAsyncWrite())
+ Q_PRIVATE_SLOT(d_func(), bool _q_startAsyncWrite())
#endif
};
diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp
index ecbf464a..18cef1ea 100644
--- a/src/serialport/qserialport_win.cpp
+++ b/src/serialport/qserialport_win.cpp
@@ -259,7 +259,7 @@ bool QSerialPortPrivate::setRequestToSend(bool set)
bool QSerialPortPrivate::flush()
{
- return startAsyncWrite();
+ return _q_startAsyncWrite();
}
bool QSerialPortPrivate::clear(QSerialPort::Directions directions)
@@ -313,7 +313,7 @@ void QSerialPortPrivate::startWriting()
if (!writeStarted) {
if (!startAsyncWriteTimer) {
startAsyncWriteTimer = new QTimer(q);
- q->connect(startAsyncWriteTimer, SIGNAL(timeout()), q, SLOT(_q_completeAsyncWrite()));
+ q->connect(startAsyncWriteTimer, SIGNAL(timeout()), q, SLOT(_q_startAsyncWrite()));
startAsyncWriteTimer->setSingleShot(true);
}
startAsyncWriteTimer->start(0);
@@ -327,7 +327,7 @@ bool QSerialPortPrivate::waitForReadyRead(int msecs)
QElapsedTimer stopWatch;
stopWatch.start();
- if (!writeStarted && !startAsyncWrite())
+ if (!writeStarted && !_q_startAsyncWrite())
return false;
const qint64 initialReadBufferSize = readBuffer.size();
@@ -381,7 +381,7 @@ bool QSerialPortPrivate::waitForBytesWritten(int msecs)
QElapsedTimer stopWatch;
stopWatch.start();
- if (!writeStarted && !startAsyncWrite())
+ if (!writeStarted && !_q_startAsyncWrite())
return false;
forever {
@@ -543,17 +543,18 @@ bool QSerialPortPrivate::_q_completeAsyncWrite()
Q_Q(QSerialPort);
if (writeStarted) {
- writeStarted = false;
const qint64 bytesTransferred = handleOverlappedResult(QSerialPort::Output, writeCompletionOverlapped);
- if (bytesTransferred == qint64(-1))
+ if (bytesTransferred == qint64(-1)) {
+ writeStarted = false;
return false;
- if (bytesTransferred > 0) {
+ } else if (bytesTransferred > 0) {
writeBuffer.free(bytesTransferred);
emit q->bytesWritten(bytesTransferred);
}
+ writeStarted = false;
}
- return startAsyncWrite();
+ return _q_startAsyncWrite();
}
bool QSerialPortPrivate::startAsyncCommunication()
@@ -605,7 +606,7 @@ bool QSerialPortPrivate::startAsyncRead()
return true;
}
-bool QSerialPortPrivate::startAsyncWrite()
+bool QSerialPortPrivate::_q_startAsyncWrite()
{
Q_Q(QSerialPort);
diff --git a/src/serialport/qserialport_win_p.h b/src/serialport/qserialport_win_p.h
index f4297ef5..cf30c189 100644
--- a/src/serialport/qserialport_win_p.h
+++ b/src/serialport/qserialport_win_p.h
@@ -108,7 +108,7 @@ public:
bool startAsyncCommunication();
bool startAsyncRead();
- bool startAsyncWrite();
+ bool _q_startAsyncWrite();
bool emulateErrorPolicy();
void emitReadyRead();
diff --git a/tests/auto/qserialport/tst_qserialport.cpp b/tests/auto/qserialport/tst_qserialport.cpp
index 9e8f6594..6fcb941b 100644
--- a/tests/auto/qserialport/tst_qserialport.cpp
+++ b/tests/auto/qserialport/tst_qserialport.cpp
@@ -46,6 +46,7 @@
Q_DECLARE_METATYPE(QSerialPort::SerialPortError);
Q_DECLARE_METATYPE(QIODevice::OpenMode);
Q_DECLARE_METATYPE(QIODevice::OpenModeFlag);
+Q_DECLARE_METATYPE(Qt::ConnectionType);
class tst_QSerialPort : public QObject
{
@@ -108,6 +109,12 @@ private slots:
void synchronousReadWrite();
+ void asynchronousWriteByBytesWritten_data();
+ void asynchronousWriteByBytesWritten();
+
+ void asynchronousWriteByTimer_data();
+ void asynchronousWriteByTimer();
+
protected slots:
void handleBytesWrittenAndExitLoopSlot(qint64 bytesWritten);
void handleBytesWrittenAndExitLoopSlot2(qint64 bytesWritten);
@@ -511,5 +518,153 @@ void tst_QSerialPort::synchronousReadWrite()
QCOMPARE(writeData, readData);
}
+class AsyncReader : public QObject
+{
+ Q_OBJECT
+public:
+ explicit AsyncReader(QSerialPort &port, Qt::ConnectionType connectionType, int expectedBytesCount)
+ : serialPort(port), expectedBytesCount(expectedBytesCount)
+ {
+ connect(&serialPort, SIGNAL(readyRead()), this, SLOT(receive()), connectionType);
+ }
+
+private slots:
+ void receive()
+ {
+ if (serialPort.bytesAvailable() < expectedBytesCount)
+ return;
+ tst_QSerialPort::exitLoop();
+ }
+
+private:
+ QSerialPort &serialPort;
+ const int expectedBytesCount;
+};
+
+class AsyncWriterByBytesWritten : public QObject
+{
+ Q_OBJECT
+public:
+ explicit AsyncWriterByBytesWritten(
+ QSerialPort &port, Qt::ConnectionType connectionType, const QByteArray &dataToWrite)
+ : serialPort(port), writeChunkSize(0)
+ {
+ writeBuffer.setData(dataToWrite);
+ writeBuffer.open(QIODevice::ReadOnly);
+ connect(&serialPort, SIGNAL(bytesWritten(qint64)), this, SLOT(send()), connectionType);
+ send();
+ }
+
+private slots:
+ void send()
+ {
+ if (writeBuffer.bytesAvailable() > 0)
+ serialPort.write(writeBuffer.read(++writeChunkSize));
+ }
+
+private:
+ QSerialPort &serialPort;
+ QBuffer writeBuffer;
+ int writeChunkSize;
+};
+
+void tst_QSerialPort::asynchronousWriteByBytesWritten_data()
+{
+ QTest::addColumn<Qt::ConnectionType>("readConnectionType");
+ QTest::addColumn<Qt::ConnectionType>("writeConnectionType");
+
+ QTest::newRow("BothQueued") << Qt::QueuedConnection << Qt::QueuedConnection;
+ QTest::newRow("BothDirect") << Qt::DirectConnection << Qt::DirectConnection;
+ QTest::newRow("ReadDirectWriteQueued") << Qt::DirectConnection << Qt::QueuedConnection;
+ QTest::newRow("ReadQueuedWriteDirect") << Qt::QueuedConnection << Qt::DirectConnection;
+}
+
+void tst_QSerialPort::asynchronousWriteByBytesWritten()
+{
+#ifdef Q_OS_WIN
+ clearReceiver();
+#endif
+
+ QFETCH(Qt::ConnectionType, readConnectionType);
+ QFETCH(Qt::ConnectionType, writeConnectionType);
+
+ QSerialPort receiverPort(m_receiverPortName);
+ QVERIFY(receiverPort.open(QSerialPort::ReadOnly));
+ AsyncReader reader(receiverPort, readConnectionType, alphabetArray.size());
+
+ QSerialPort senderPort(m_senderPortName);
+ QVERIFY(senderPort.open(QSerialPort::WriteOnly));
+ AsyncWriterByBytesWritten writer(senderPort, writeConnectionType, alphabetArray);
+
+ enterLoop(1);
+ QVERIFY2(!timeout(), "Timed out when waiting for the read or write.");
+ QCOMPARE(receiverPort.bytesAvailable(), qint64(alphabetArray.size()));
+ QCOMPARE(receiverPort.readAll(), alphabetArray);
+}
+
+class AsyncWriterByTimer : public QObject
+{
+ Q_OBJECT
+public:
+ explicit AsyncWriterByTimer(
+ QSerialPort &port, Qt::ConnectionType connectionType, const QByteArray &dataToWrite)
+ : serialPort(port), writeChunkSize(0)
+ {
+ writeBuffer.setData(dataToWrite);
+ writeBuffer.open(QIODevice::ReadOnly);
+ connect(&timer, SIGNAL(timeout()), this, SLOT(send()), connectionType);
+ timer.start(0);
+ }
+
+private slots:
+ void send()
+ {
+ if (writeBuffer.bytesAvailable() > 0)
+ serialPort.write(writeBuffer.read(++writeChunkSize));
+ else
+ timer.stop();
+ }
+
+private:
+ QSerialPort &serialPort;
+ QBuffer writeBuffer;
+ int writeChunkSize;
+ QTimer timer;
+};
+
+void tst_QSerialPort::asynchronousWriteByTimer_data()
+{
+ QTest::addColumn<Qt::ConnectionType>("readConnectionType");
+ QTest::addColumn<Qt::ConnectionType>("writeConnectionType");
+
+ QTest::newRow("BothQueued") << Qt::QueuedConnection << Qt::QueuedConnection;
+ QTest::newRow("BothDirect") << Qt::DirectConnection << Qt::DirectConnection;
+ QTest::newRow("ReadDirectWriteQueued") << Qt::DirectConnection << Qt::QueuedConnection;
+ QTest::newRow("ReadQueuedWriteDirect") << Qt::QueuedConnection << Qt::DirectConnection;
+}
+
+void tst_QSerialPort::asynchronousWriteByTimer()
+{
+#ifdef Q_OS_WIN
+ clearReceiver();
+#endif
+
+ QFETCH(Qt::ConnectionType, readConnectionType);
+ QFETCH(Qt::ConnectionType, writeConnectionType);
+
+ QSerialPort receiverPort(m_receiverPortName);
+ QVERIFY(receiverPort.open(QSerialPort::ReadOnly));
+ AsyncReader reader(receiverPort, readConnectionType, alphabetArray.size());
+
+ QSerialPort senderPort(m_senderPortName);
+ QVERIFY(senderPort.open(QSerialPort::WriteOnly));
+ AsyncWriterByTimer writer(senderPort, writeConnectionType, alphabetArray);
+
+ enterLoop(1);
+ QVERIFY2(!timeout(), "Timed out when waiting for the read or write.");
+ QCOMPARE(receiverPort.bytesAvailable(), qint64(alphabetArray.size()));
+ QCOMPARE(receiverPort.readAll(), alphabetArray);
+}
+
QTEST_MAIN(tst_QSerialPort)
#include "tst_qserialport.moc"