From 91a3ba14961965cb80c7fdcad4a3473b2e7403ea Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 10 Jul 2023 09:47:22 -0700 Subject: [PATCH] QSocketNotifier: firm up the ordering in unexpectedDisconnection() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test was relying on the fact that, having written 1 byte to both writeEnd1 and writeEnd2 (and ensured those bytes were written with waitForBytesWritten()), both read ends would be activated by the next event loop. It turns out that this was an unreliable assumption, because the processing of that 1 byte on the second socket may not have happened yet. So firm up by waiting that both read ends are readable before even creating the QSocketNotifiers we will read on. I'm not entirely sure what this test is attempting to test. Its documentation says it's testing a QAbstractSocket condition, but the read ends aren't QAbstractSocket (this test should have been in tst_QAbstractSocket if so). It may be testing the condition that caused that QAbstractSocket behavior, but that wouldn't be a good test. Drive-by remove redundant flush()-after-waitForBytesWritten() calls. Fixes: QTBUG-115154 Change-Id: I61b74deaf2514644a24efffd17708f8071f707ed Reviewed-by: Mårten Nordheim Reviewed-by: Qt CI Bot (cherry picked from commit 93b87b5cbfae3c50e539f6ec37bd7b95e89e455e) Reviewed-by: Qt Cherry-pick Bot --- tests/auto/corelib/kernel/qsocketnotifier/BLACKLIST | 2 -- .../kernel/qsocketnotifier/tst_qsocketnotifier.cpp | 13 +++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) delete mode 100644 tests/auto/corelib/kernel/qsocketnotifier/BLACKLIST diff --git a/tests/auto/corelib/kernel/qsocketnotifier/BLACKLIST b/tests/auto/corelib/kernel/qsocketnotifier/BLACKLIST deleted file mode 100644 index e23fe96ba30..00000000000 --- a/tests/auto/corelib/kernel/qsocketnotifier/BLACKLIST +++ /dev/null @@ -1,2 +0,0 @@ -[unexpectedDisconnection] -macos ci # QTBUG-115154 diff --git a/tests/auto/corelib/kernel/qsocketnotifier/tst_qsocketnotifier.cpp b/tests/auto/corelib/kernel/qsocketnotifier/tst_qsocketnotifier.cpp index 2e7d1767fde..641b74bd9f0 100644 --- a/tests/auto/corelib/kernel/qsocketnotifier/tst_qsocketnotifier.cpp +++ b/tests/auto/corelib/kernel/qsocketnotifier/tst_qsocketnotifier.cpp @@ -132,11 +132,11 @@ public slots: ++sequence; if (sequence == 1) { // read from both ends - (void) readEnd1->read(data1, sizeof(data1)); - (void) readEnd2->read(data2, sizeof(data2)); + QCOMPARE(readEnd1->read(data1, sizeof(data1)), 1); + QCOMPARE(readEnd2->read(data2, sizeof(data2)), 1); emit finished(); } else if (sequence == 2) { - // we should never get here + // check that we can't read now because we've read our byte QCOMPARE(readEnd2->read(data2, sizeof(data2)), qint64(-2)); QVERIFY(readEnd2->isValid()); } @@ -152,7 +152,7 @@ void tst_QSocketNotifier::unexpectedDisconnection() Given two sockets and two QSocketNotifiers registered on each their socket. If both sockets receive data, and the first slot invoked by one of the socket notifiers empties both sockets, the - other notifier will also emit activated(). This results in + other notifier will also emit activated(). This was causing an unexpected disconnection in QAbstractSocket. The use case is that somebody calls one of the @@ -188,8 +188,9 @@ void tst_QSocketNotifier::unexpectedDisconnection() writeEnd1->waitForBytesWritten(); writeEnd2->waitForBytesWritten(); - writeEnd1->flush(); - writeEnd2->flush(); + // ensure both read ends are ready for reading, before the event loop + QVERIFY(readEnd1.waitForRead(5000)); + QVERIFY(readEnd2.waitForRead(5000)); UnexpectedDisconnectTester tester(&readEnd1, &readEnd2);