From 47bf28ee3a98aa29901eb937cd4d23ab889d7595 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Wed, 19 Dec 2018 12:27:04 +0100 Subject: [PATCH] QProcess/Win: Use synchronous writes for the stdin pipes Windows PowerShell scripts that try to read from stdin using [Console]::in.ReadLine() hang in the ReadLine call unless the FILE_FLAG_OVERLAPPED flag for stdin is dropped. Consequently, this patch make the pipe handle for stdin synchronous and replaces the asynchronous QWindowsPipeWriter with the old synchronous implementation, renamed to QWindowsSynchronousPipeWriter. Fixes: QTBUG-68169 Change-Id: Icb1a03a72a9995849309042e0597d2926c5c162a --- src/corelib/io/io.pri | 6 +- src/corelib/io/qprocess_p.h | 4 +- src/corelib/io/qprocess_win.cpp | 25 +-- src/corelib/io/qwindowspipewriter.cpp | 2 + src/corelib/io/qwindowspipewriter_p.h | 50 ----- .../io/qwindowssynchronouspipewriter.cpp | 178 ++++++++++++++++++ .../io/qwindowssynchronouspipewriter_p.h | 154 +++++++++++++++ 7 files changed, 354 insertions(+), 65 deletions(-) create mode 100644 src/corelib/io/qwindowssynchronouspipewriter.cpp create mode 100644 src/corelib/io/qwindowssynchronouspipewriter_p.h diff --git a/src/corelib/io/io.pri b/src/corelib/io/io.pri index 086d642c26..edf6e0bfb3 100644 --- a/src/corelib/io/io.pri +++ b/src/corelib/io/io.pri @@ -148,13 +148,15 @@ win32 { !winrt { HEADERS += \ io/qwindowspipereader_p.h \ - io/qwindowspipewriter_p.h + io/qwindowspipewriter_p.h \ + io/qwindowssynchronouspipewriter_p.h SOURCES += \ io/qstandardpaths_win.cpp \ io/qstorageinfo_win.cpp \ io/qwindowspipereader.cpp \ - io/qwindowspipewriter.cpp + io/qwindowspipewriter.cpp \ + io/qwindowssynchronouspipewriter.cpp LIBS += -lmpr -lnetapi32 -luserenv } else { diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index aa7ecbe91d..c3ac8141ca 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -78,7 +78,7 @@ QT_BEGIN_NAMESPACE class QSocketNotifier; class QWindowsPipeReader; -class QWindowsPipeWriter; +class QWindowsSynchronousPipeWriter; class QWinEventNotifier; class QTimer; @@ -286,7 +286,7 @@ public: #ifdef Q_OS_WIN union { QWindowsPipeReader *reader; - QWindowsPipeWriter *writer; + QWindowsSynchronousPipeWriter *writer; }; #endif Q_PIPE pipe[2]; diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp index cb4b2f9f91..6bb12a8778 100644 --- a/src/corelib/io/qprocess_win.cpp +++ b/src/corelib/io/qprocess_win.cpp @@ -42,7 +42,7 @@ #include "qprocess.h" #include "qprocess_p.h" #include "qwindowspipereader_p.h" -#include "qwindowspipewriter_p.h" +#include "qwindowssynchronouspipewriter_p.h" #include #include @@ -99,12 +99,13 @@ static void qt_create_pipe(Q_PIPE *pipe, bool isInputPipe) HANDLE hServer; wchar_t pipeName[256]; unsigned int attempts = 1000; + DWORD maybeOverlappedFlag = isInputPipe ? 0 : FILE_FLAG_OVERLAPPED; forever { _snwprintf(pipeName, sizeof(pipeName) / sizeof(pipeName[0]), L"\\\\.\\pipe\\qt-%lX-%X", long(QCoreApplication::applicationPid()), QRandomGenerator::global()->generate()); - DWORD dwOpenMode = FILE_FLAG_OVERLAPPED; + DWORD dwOpenMode = maybeOverlappedFlag; DWORD dwOutputBufferSize = 0; DWORD dwInputBufferSize = 0; const DWORD dwPipeBufferSize = 1024 * 1024; @@ -140,7 +141,7 @@ static void qt_create_pipe(Q_PIPE *pipe, bool isInputPipe) 0, &secAtt, OPEN_EXISTING, - FILE_FLAG_OVERLAPPED, + maybeOverlappedFlag, NULL); if (hClient == INVALID_HANDLE_VALUE) { qErrnoWarning("QProcess: CreateFile failed."); @@ -716,7 +717,10 @@ bool QProcessPrivate::waitForBytesWritten(int msecs) QIncrementalSleepTimer timer(msecs); forever { - bool pendingDataInPipe = stdinChannel.writer && stdinChannel.writer->bytesToWrite(); + // Check if we have any data pending: the pipe writer has + // bytes waiting to written, or it has written data since the + // last time we called stdinChannel.writer->waitForWrite(). + bool pendingDataInPipe = stdinChannel.writer && (stdinChannel.writer->bytesToWrite() || stdinChannel.writer->hadWritten()); // If we don't have pending data, and our write buffer is // empty, we fail. @@ -841,17 +845,16 @@ bool QProcessPrivate::writeToStdin() Q_Q(QProcess); if (!stdinChannel.writer) { - stdinChannel.writer = new QWindowsPipeWriter(stdinChannel.pipe[1], q); - QObject::connect(stdinChannel.writer, &QWindowsPipeWriter::bytesWritten, + stdinChannel.writer = new QWindowsSynchronousPipeWriter(stdinChannel.pipe[1], q); + QObject::connect(stdinChannel.writer, &QWindowsSynchronousPipeWriter::bytesWritten, q, &QProcess::bytesWritten); - QObjectPrivate::connect(stdinChannel.writer, &QWindowsPipeWriter::canWrite, + QObjectPrivate::connect(stdinChannel.writer, &QWindowsSynchronousPipeWriter::canWrite, this, &QProcessPrivate::_q_canWrite); - } else { - if (stdinChannel.writer->isWriteOperationActive()) - return true; + stdinChannel.writer->start(); } - stdinChannel.writer->write(writeBuffer.read()); + const QByteArray &data = writeBuffer.read(); + stdinChannel.writer->write(data.constData(), data.count()); return true; } diff --git a/src/corelib/io/qwindowspipewriter.cpp b/src/corelib/io/qwindowspipewriter.cpp index 92e8b6db52..2317e92f78 100644 --- a/src/corelib/io/qwindowspipewriter.cpp +++ b/src/corelib/io/qwindowspipewriter.cpp @@ -40,6 +40,8 @@ #include "qwindowspipewriter_p.h" #include "qiodevice_p.h" +#include + QT_BEGIN_NAMESPACE QWindowsPipeWriter::Overlapped::Overlapped(QWindowsPipeWriter *pipeWriter) diff --git a/src/corelib/io/qwindowspipewriter_p.h b/src/corelib/io/qwindowspipewriter_p.h index d6671c3f27..8bee2b93ad 100644 --- a/src/corelib/io/qwindowspipewriter_p.h +++ b/src/corelib/io/qwindowspipewriter_p.h @@ -51,62 +51,12 @@ // We mean it. // -#include -#include #include #include #include QT_BEGIN_NAMESPACE -#define SLEEPMIN 10 -#define SLEEPMAX 500 - -class QIncrementalSleepTimer -{ - -public: - QIncrementalSleepTimer(int msecs) - : totalTimeOut(msecs) - , nextSleep(qMin(SLEEPMIN, totalTimeOut)) - { - if (totalTimeOut == -1) - nextSleep = SLEEPMIN; - timer.start(); - } - - int nextSleepTime() - { - int tmp = nextSleep; - nextSleep = qMin(nextSleep * 2, qMin(SLEEPMAX, timeLeft())); - return tmp; - } - - int timeLeft() const - { - if (totalTimeOut == -1) - return SLEEPMAX; - return qMax(int(totalTimeOut - timer.elapsed()), 0); - } - - bool hasTimedOut() const - { - if (totalTimeOut == -1) - return false; - return timer.elapsed() >= totalTimeOut; - } - - void resetIncrements() - { - nextSleep = qMin(SLEEPMIN, timeLeft()); - } - -private: - QElapsedTimer timer; - int totalTimeOut; - int nextSleep; -}; - class Q_CORE_EXPORT QWindowsPipeWriter : public QObject { Q_OBJECT diff --git a/src/corelib/io/qwindowssynchronouspipewriter.cpp b/src/corelib/io/qwindowssynchronouspipewriter.cpp new file mode 100644 index 0000000000..6bb2a11228 --- /dev/null +++ b/src/corelib/io/qwindowssynchronouspipewriter.cpp @@ -0,0 +1,178 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtCore 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 The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/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 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include "qwindowssynchronouspipewriter_p.h" + +QT_BEGIN_NAMESPACE + +#ifndef QT_NO_THREAD + +QWindowsSynchronousPipeWriter::QWindowsSynchronousPipeWriter(HANDLE pipe, QObject * parent) + : QThread(parent), + writePipe(pipe), + quitNow(false), + hasWritten(false) +{ +} + +QWindowsSynchronousPipeWriter::~QWindowsSynchronousPipeWriter() +{ + lock.lock(); + quitNow = true; + waitCondition.wakeOne(); + lock.unlock(); + if (!wait(30000)) + terminate(); +} + +bool QWindowsSynchronousPipeWriter::waitForWrite(int msecs) +{ + QMutexLocker locker(&lock); + bool hadWritten = hasWritten; + hasWritten = false; + if (hadWritten) + return true; + if (!waitCondition.wait(&lock, msecs)) + return false; + hadWritten = hasWritten; + hasWritten = false; + return hadWritten; +} + +qint64 QWindowsSynchronousPipeWriter::write(const char *ptr, qint64 maxlen) +{ + if (!isRunning()) + return -1; + + QMutexLocker locker(&lock); + data.append(ptr, maxlen); + waitCondition.wakeOne(); + return maxlen; +} + +class QPipeWriterOverlapped +{ +public: + QPipeWriterOverlapped() + { + overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); + } + + ~QPipeWriterOverlapped() + { + CloseHandle(overlapped.hEvent); + } + + void prepare() + { + const HANDLE hEvent = overlapped.hEvent; + ZeroMemory(&overlapped, sizeof overlapped); + overlapped.hEvent = hEvent; + } + + OVERLAPPED *operator&() + { + return &overlapped; + } + +private: + OVERLAPPED overlapped; +}; + +void QWindowsSynchronousPipeWriter::run() +{ + QPipeWriterOverlapped overl; + forever { + lock.lock(); + while(data.isEmpty() && (!quitNow)) { + waitCondition.wakeOne(); + waitCondition.wait(&lock); + } + + if (quitNow) { + lock.unlock(); + quitNow = false; + break; + } + + QByteArray copy = data; + + lock.unlock(); + + const char *ptrData = copy.data(); + qint64 maxlen = copy.size(); + qint64 totalWritten = 0; + overl.prepare(); + while ((!quitNow) && totalWritten < maxlen) { + DWORD written = 0; + if (!WriteFile(writePipe, ptrData + totalWritten, + maxlen - totalWritten, &written, &overl)) { + const DWORD writeError = GetLastError(); + if (writeError == 0xE8/*NT_STATUS_INVALID_USER_BUFFER*/) { + // give the os a rest + msleep(100); + continue; + } + if (writeError != ERROR_IO_PENDING) { + qErrnoWarning(writeError, "QWindowsSynchronousPipeWriter: async WriteFile failed."); + return; + } + if (!GetOverlappedResult(writePipe, &overl, &written, TRUE)) { + qErrnoWarning(GetLastError(), "QWindowsSynchronousPipeWriter: GetOverlappedResult failed."); + return; + } + } + totalWritten += written; +#if defined QPIPEWRITER_DEBUG + qDebug("QWindowsSynchronousPipeWriter::run() wrote %d %d/%d bytes", + written, int(totalWritten), int(maxlen)); +#endif + lock.lock(); + data.remove(0, written); + hasWritten = true; + lock.unlock(); + } + emit bytesWritten(totalWritten); + emit canWrite(); + } +} + +#endif //QT_NO_THREAD + +QT_END_NAMESPACE diff --git a/src/corelib/io/qwindowssynchronouspipewriter_p.h b/src/corelib/io/qwindowssynchronouspipewriter_p.h new file mode 100644 index 0000000000..60d7fe93b3 --- /dev/null +++ b/src/corelib/io/qwindowssynchronouspipewriter_p.h @@ -0,0 +1,154 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtCore 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 The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/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 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef QWINDOWSPIPEWRITER_P_H +#define QWINDOWSPIPEWRITER_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// + +#include +#include +#include +#include +#include + +QT_BEGIN_NAMESPACE + +#ifndef QT_NO_THREAD + +#define SLEEPMIN 10 +#define SLEEPMAX 500 + +class QIncrementalSleepTimer +{ +public: + QIncrementalSleepTimer(int msecs) + : totalTimeOut(msecs) + , nextSleep(qMin(SLEEPMIN, totalTimeOut)) + { + if (totalTimeOut == -1) + nextSleep = SLEEPMIN; + timer.start(); + } + + int nextSleepTime() + { + int tmp = nextSleep; + nextSleep = qMin(nextSleep * 2, qMin(SLEEPMAX, timeLeft())); + return tmp; + } + + int timeLeft() const + { + if (totalTimeOut == -1) + return SLEEPMAX; + return qMax(int(totalTimeOut - timer.elapsed()), 0); + } + + bool hasTimedOut() const + { + if (totalTimeOut == -1) + return false; + return timer.elapsed() >= totalTimeOut; + } + + void resetIncrements() + { + nextSleep = qMin(SLEEPMIN, timeLeft()); + } + +private: + QElapsedTimer timer; + int totalTimeOut; + int nextSleep; +}; + + +class QWindowsSynchronousPipeWriter : public QThread +{ + Q_OBJECT + +Q_SIGNALS: + void canWrite(); + void bytesWritten(qint64 bytes); + +public: + explicit QWindowsSynchronousPipeWriter(HANDLE writePipe, QObject * parent = 0); + ~QWindowsSynchronousPipeWriter(); + + bool waitForWrite(int msecs); + qint64 write(const char *data, qint64 maxlen); + + qint64 bytesToWrite() const + { + QMutexLocker locker(&lock); + return data.size(); + } + + bool hadWritten() const + { + return hasWritten; + } + +protected: + void run(); + +private: + QByteArray data; + QWaitCondition waitCondition; + mutable QMutex lock; + HANDLE writePipe; + volatile bool quitNow; + bool hasWritten; +}; + +#endif //QT_NO_THREAD + +QT_END_NAMESPACE + +#endif // QT_NO_PROCESS -- 2.19.1.windows.1