Refactor openMode/keepWriteChannelOpen/closeWriteChannel

Change-Id: I4090533875ce1da864b6f8554ce59dbc1392a142
Reviewed-by: hjk <hjk@qt.io>

src/libs/utils/CMakeLists.txt

@@ -123,6 +123,7 @@ add_qtc_library(Utils
     portlist.cpp portlist.h
     predicates.h
     processhandle.cpp processhandle.h
+    processutils.cpp processutils.h
     progressindicator.cpp progressindicator.h
     projectintropage.cpp projectintropage.h projectintropage.ui
     proxyaction.cpp proxyaction.h

src/libs/utils/launcherpackets.cpp

@@ -58,12 +58,14 @@ StartProcessPacket::StartProcessPacket(quintptr token)
 
 void StartProcessPacket::doSerialize(QDataStream &stream) const
 {
-    stream << command << arguments << workingDir << env << openMode << channelMode << standardInputFile;
+    stream << command << arguments << workingDir << env << processMode << writeData << channelMode
+           << standardInputFile;
 }
 
 void StartProcessPacket::doDeserialize(QDataStream &stream)
 {
-    stream >> command >> arguments >> workingDir >> env >> openMode >> channelMode >> standardInputFile;
+    stream >> command >> arguments >> workingDir >> env >> processMode >> writeData >> channelMode
+           >> standardInputFile;
 }
 
 

src/libs/utils/launcherpackets.h

@@ -25,6 +25,8 @@
 
 #pragma once
 
+#include "processutils.h"
+
 #include <QtCore/qdatastream.h>
 #include <QtCore/qprocess.h>
 #include <QtCore/qstringlist.h>
@@ -109,7 +111,8 @@ public:
     QStringList arguments;
     QString workingDir;
     QStringList env;
-    QIODevice::OpenMode openMode = QIODevice::ReadWrite;
+    ProcessMode processMode = ProcessMode::Reader;
+    QByteArray writeData;
     QProcess::ProcessChannelMode channelMode = QProcess::SeparateChannels;
     QString standardInputFile;
 

src/libs/utils/launchersocket.cpp

@@ -314,7 +314,7 @@ void LauncherHandle::cancel()
     m_processState = QProcess::NotRunning;
 }
 
-void LauncherHandle::start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode)
+void LauncherHandle::start(const QString &program, const QStringList &arguments, const QByteArray &writeData)
 {
     QMutexLocker locker(&m_mutex);
 
@@ -328,7 +328,7 @@ void LauncherHandle::start(const QString &program, const QStringList &arguments,
     // TODO: check if state is not running
     // TODO: check if m_canceled is not true
     m_processState = QProcess::Starting;
-    m_openMode = mode;
+    m_writeData = writeData;
     if (LauncherInterface::socket()->isReady())
         doStart();
 }
@@ -405,7 +405,8 @@ void LauncherHandle::doStart()
     p.arguments = m_arguments;
     p.env = m_environment.toStringList();
     p.workingDir = m_workingDirectory;
-    p.openMode = m_openMode;
+    p.processMode = m_processMode;
+    p.writeData = m_writeData;
     p.channelMode = m_channelMode;
     p.standardInputFile = m_standardInputFile;
     sendPacket(p);
@@ -427,13 +428,13 @@ void LauncherSocket::sendData(const QByteArray &data)
         QMetaObject::invokeMethod(this, &LauncherSocket::handleRequests);
 }
 
-LauncherHandle *LauncherSocket::registerHandle(quintptr token)
+LauncherHandle *LauncherSocket::registerHandle(quintptr token, ProcessMode mode)
 {
     QMutexLocker locker(&m_mutex);
     if (m_handles.contains(token))
         return nullptr; // TODO: issue a warning
 
-    LauncherHandle *handle = new LauncherHandle(token);
+    LauncherHandle *handle = new LauncherHandle(token, mode);
     handle->moveToThread(thread());
     // Call it after moving LauncherHandle to the launcher's thread.
     // Since this method is invoked from caller's thread, CallerHandle will live in caller's thread.

src/libs/utils/launchersocket.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include "launcherpackets.h"
+#include "processutils.h"
 
 #include <QtCore/qobject.h>
 
@@ -82,12 +83,7 @@ public:
     QString errorString() const { QMutexLocker locker(&m_mutex); return m_errorString; }
     void setErrorString(const QString &str) { QMutexLocker locker(&m_mutex); m_errorString = str; }
 
-    // Called from other thread. Create a temp object receiver which lives in caller's thread.
-    // Add started and finished signals to it and post a flush to it.
-    // When we are in waitForSignal() which is called from the same thread,
-    // we may flush the signal queue and emit these signals immediately.
-    // Who should remove this object? deleteLater()?
-    void start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode);
+    void start(const QString &program, const QStringList &arguments, const QByteArray &writeData);
 
     qint64 write(const QByteArray &data);
 
@@ -131,7 +127,7 @@ private:
     void slotFinished();
 
     // called from this thread
-    LauncherHandle(quintptr token) : m_token(token) {}
+    LauncherHandle(quintptr token, ProcessMode mode) : m_token(token), m_processMode(mode) {}
     void createCallerHandle();
     void destroyCallerHandle();
 
@@ -161,6 +157,7 @@ private:
     mutable QMutex m_mutex;
     QWaitCondition m_waitCondition;
     const quintptr m_token;
+    const ProcessMode m_processMode;
     SignalType m_waitingFor = SignalType::NoSignal;
 
     QProcess::ProcessState m_processState = QProcess::NotRunning;
@@ -178,7 +175,7 @@ private:
     QStringList m_arguments;
     QProcessEnvironment m_environment;
     QString m_workingDirectory;
-    QIODevice::OpenMode m_openMode = QIODevice::ReadWrite;
+    QByteArray m_writeData;
     QProcess::ProcessChannelMode m_channelMode = QProcess::SeparateChannels;
     QString m_standardInputFile;
 
@@ -196,7 +193,7 @@ public:
     bool isReady() const { return m_socket.load(); }
     void sendData(const QByteArray &data);
 
-    LauncherHandle *registerHandle(quintptr token);
+    LauncherHandle *registerHandle(quintptr token, ProcessMode mode);
     void unregisterHandle(quintptr token);
 
 signals:

src/libs/utils/processutils.cpp

@@ -0,0 +1,58 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** 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 General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** 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-3.0.html.
+**
+****************************************************************************/
+
+#include "processutils.h"
+#include <QProcess>
+
+namespace Utils {
+
+QIODevice::OpenMode ProcessStartHandler::openMode() const
+{
+    if (m_processMode == ProcessMode::Writer)
+        return QIODevice::ReadWrite; // some writers also read data
+    if (m_writeData.isEmpty())
+        return QIODevice::ReadOnly; // only reading
+    return QIODevice::ReadWrite; // initial write and then reading (close the write channel)
+}
+
+void ProcessStartHandler::handleProcessStart(QProcess *process)
+{
+    if (m_processMode == ProcessMode::Writer)
+        return;
+    if (m_writeData.isEmpty())
+        process->closeWriteChannel();
+}
+
+void ProcessStartHandler::handleProcessStarted(QProcess *process)
+{
+    if (!m_writeData.isEmpty()) {
+        process->write(m_writeData);
+        m_writeData = {};
+        if (m_processMode == ProcessMode::Reader)
+            process->closeWriteChannel();
+    }
+}
+
+} // namespace Utils

src/libs/utils/processutils.h

@@ -0,0 +1,58 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of Qt Creator.
+**
+** 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 General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** 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-3.0.html.
+**
+****************************************************************************/
+
+#pragma once
+
+#include <QIODevice>
+
+QT_BEGIN_NAMESPACE
+class QProcess;
+QT_END_NAMESPACE
+
+namespace Utils {
+
+enum class ProcessMode {
+    Reader, // This opens in ReadOnly mode if no write data or in ReadWrite mode otherwise,
+            // closes the write channel afterwards
+    Writer  // This opens in ReadWrite mode and doesn't close the write channel
+};
+
+class ProcessStartHandler {
+public:
+    void setProcessMode(ProcessMode mode) { m_processMode = mode; }
+    void setWriteData(const QByteArray &writeData) { m_writeData = writeData; }
+    QIODevice::OpenMode openMode() const;
+    void handleProcessStart(QProcess *process);
+    void handleProcessStarted(QProcess *process);
+
+private:
+    ProcessMode m_processMode = ProcessMode::Reader;
+    QByteArray m_writeData;
+};
+
+
+} // namespace Utils
+
+Q_DECLARE_METATYPE(Utils::ProcessMode);

src/libs/utils/qtcprocess.cpp

@@ -101,14 +101,17 @@ class ProcessInterface : public QObject
 {
     Q_OBJECT
 public:
-    ProcessInterface() : QObject() {}
+    ProcessInterface(ProcessMode processMode)
+        : QObject()
+        , m_processMode(processMode) {}
 
     virtual QByteArray readAllStandardOutput() = 0;
     virtual QByteArray readAllStandardError() = 0;
 
     virtual void setProcessEnvironment(const QProcessEnvironment &environment) = 0;
     virtual void setWorkingDirectory(const QString &dir) = 0;
-    virtual void start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode) = 0;
+    virtual void start(const QString &program, const QStringList &arguments,
+                       const QByteArray &writeData) = 0;
     virtual void terminate() = 0;
     virtual void kill() = 0;
     virtual void close() = 0;
@@ -145,6 +148,11 @@ signals:
     void errorOccurred(QProcess::ProcessError error);
     void readyReadStandardOutput();
     void readyReadStandardError();
+
+protected:
+    ProcessMode processMode() const { return m_processMode; }
+private:
+    const ProcessMode m_processMode;
 };
 
 class ProcessHelper : public QProcess
@@ -186,10 +194,10 @@ public:
 class QProcessImpl : public ProcessInterface
 {
 public:
-    QProcessImpl() : ProcessInterface()
+    QProcessImpl(ProcessMode processMode) : ProcessInterface(processMode)
     {
         connect(&m_process, &QProcess::started,
-                this, &ProcessInterface::started);
+                this, &QProcessImpl::handleStarted);
         connect(&m_process, QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
                 this, &ProcessInterface::finished);
         connect(&m_process, &QProcess::errorOccurred,
@@ -207,8 +215,13 @@ public:
     { m_process.setProcessEnvironment(environment); }
     void setWorkingDirectory(const QString &dir) override
     { m_process.setWorkingDirectory(dir); }
-    void start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode) override
-    { m_process.start(program, arguments, mode); }
+    void start(const QString &program, const QStringList &arguments, const QByteArray &writeData) override
+    {
+        m_processStartHandler.setProcessMode(processMode());
+        m_processStartHandler.setWriteData(writeData);
+        m_process.start(program, arguments, m_processStartHandler.openMode());
+        m_processStartHandler.handleProcessStart(&m_process);
+    }
     void terminate() override
     { m_process.terminate(); }
     void kill() override
@@ -262,7 +275,13 @@ public:
 #endif
 
 private:
+    void handleStarted()
+    {
+        m_processStartHandler.handleProcessStarted(&m_process);
+        emit started();
+    }
     ProcessHelper m_process;
+    ProcessStartHandler m_processStartHandler;
 };
 
 static uint uniqueToken()
@@ -275,9 +294,10 @@ class ProcessLauncherImpl : public ProcessInterface
 {
     Q_OBJECT
 public:
-    ProcessLauncherImpl() : ProcessInterface(), m_token(uniqueToken())
+    ProcessLauncherImpl(ProcessMode processMode)
+        : ProcessInterface(processMode), m_token(uniqueToken())
     {
-        m_handle = LauncherInterface::socket()->registerHandle(token());
+        m_handle = LauncherInterface::socket()->registerHandle(token(), processMode);
         connect(m_handle, &LauncherHandle::errorOccurred,
                 this, &ProcessInterface::errorOccurred);
         connect(m_handle, &LauncherHandle::started,
@@ -301,8 +321,8 @@ public:
     void setProcessEnvironment(const QProcessEnvironment &environment) override
     { m_handle->setProcessEnvironment(environment); }
     void setWorkingDirectory(const QString &dir) override { m_handle->setWorkingDirectory(dir); }
-    void start(const QString &program, const QStringList &arguments, QIODevice::OpenMode mode) override
-    { m_handle->start(program, arguments, mode); }
+    void start(const QString &program, const QStringList &arguments, const QByteArray &writeData) override
+    { m_handle->start(program, arguments, writeData); }
     void terminate() override { cancel(); } // TODO: what are differences among terminate, kill and close?
     void kill() override { cancel(); } // TODO: see above
     void close() override { cancel(); } // TODO: see above
@@ -355,18 +375,20 @@ void ProcessLauncherImpl::cancel()
     m_handle->cancel();
 }
 
-static ProcessInterface *newProcessInstance(QtcProcess::ProcessImpl processImpl)
+static ProcessInterface *newProcessInstance(QtcProcess::ProcessImpl processImpl, ProcessMode mode)
 {
     if (processImpl == QtcProcess::QProcessImpl)
-        return new QProcessImpl;
-    return new ProcessLauncherImpl;
+        return new QProcessImpl(mode);
+    return new ProcessLauncherImpl(mode);
 }
 
 class QtcProcessPrivate : public QObject
 {
 public:
-    explicit QtcProcessPrivate(QtcProcess *parent, QtcProcess::ProcessImpl processImpl)
-        : q(parent), m_process(newProcessInstance(processImpl))
+    explicit QtcProcessPrivate(QtcProcess *parent,
+                               QtcProcess::ProcessImpl processImpl,
+                               ProcessMode processMode)
+        : q(parent), m_process(newProcessInstance(processImpl, processMode)), m_processMode(processMode)
     {
         connect(m_process, &ProcessInterface::started,
                 q, &QtcProcess::started);
@@ -403,6 +425,7 @@ public:
 
     QtcProcess *q;
     ProcessInterface *m_process;
+    const ProcessMode m_processMode;
     CommandLine m_commandLine;
     FilePath m_workingDirectory;
     Environment m_environment;
@@ -469,8 +492,8 @@ QtcProcess::Result QtcProcessPrivate::interpretExitCode(int exitCode)
     \sa Utils::ProcessArgs
 */
 
-QtcProcess::QtcProcess(ProcessImpl processImpl, QObject *parent)
-    : QObject(parent), d(new QtcProcessPrivate(this, processImpl))
+QtcProcess::QtcProcess(ProcessImpl processImpl, ProcessMode processMode, QObject *parent)
+    : QObject(parent), d(new QtcProcessPrivate(this, processImpl, processMode))
 {
     static int qProcessExitStatusMeta = qRegisterMetaType<QProcess::ExitStatus>();
     static int qProcessProcessErrorMeta = qRegisterMetaType<QProcess::ProcessError>();
@@ -478,7 +501,11 @@ QtcProcess::QtcProcess(ProcessImpl processImpl, QObject *parent)
     Q_UNUSED(qProcessProcessErrorMeta)
 }
 
-QtcProcess::QtcProcess(QObject *parent) : QtcProcess(QtcProcess::QProcessImpl, parent) {}
+QtcProcess::QtcProcess(ProcessMode processMode, QObject *parent)
+    : QtcProcess(QtcProcess::QProcessImpl, processMode, parent) {}
+
+QtcProcess::QtcProcess(QObject *parent)
+    : QtcProcess(QtcProcess::QProcessImpl, ProcessMode::Reader, parent) {}
 
 QtcProcess::~QtcProcess()
 {
@@ -615,7 +642,7 @@ void QtcProcess::start()
 #endif
         // Note: Arguments set with setNativeArgs will be appended to the ones
         // passed with start() below.
-        d->m_process->start(command, QStringList(), d->m_openMode);
+        d->m_process->start(command, QStringList(), d->m_writeData);
     } else {
         if (!success) {
             setErrorString(tr("Error in command line."));
@@ -624,7 +651,7 @@ void QtcProcess::start()
             emit errorOccurred(QProcess::UnknownError);
             return;
         }
-        d->m_process->start(command, arguments.toUnixArgs(), d->m_openMode);
+        d->m_process->start(command, arguments.toUnixArgs(), d->m_writeData);
     }
 }
 

src/libs/utils/qtcprocess.h

@@ -29,6 +29,7 @@
 
 #include "environment.h"
 #include "commandline.h"
+#include "processutils.h"
 
 #include <QProcess>
 #include <QTextCodec>
@@ -64,7 +65,8 @@ public:
         ProcessLauncherImpl
     };
 
-    QtcProcess(ProcessImpl processImpl, QObject *parent = nullptr);
+    QtcProcess(ProcessImpl processImpl, ProcessMode processMode, QObject *parent = nullptr);
+    QtcProcess(ProcessMode processMode, QObject *parent = nullptr);
     QtcProcess(QObject *parent = nullptr);
     ~QtcProcess();
 

src/libs/utils/utils-lib.pri

@@ -93,6 +93,7 @@ SOURCES += \
     $$PWD/json.cpp \
     $$PWD/portlist.cpp \
     $$PWD/processhandle.cpp \
+    $$PWD/processutils.cpp \
     $$PWD/appmainwindow.cpp \
     $$PWD/basetreeview.cpp \
     $$PWD/qtcassert.cpp \
@@ -230,6 +231,7 @@ HEADERS += \
     $$PWD/runextensions.h \
     $$PWD/portlist.h \
     $$PWD/processhandle.h \
+    $$PWD/processutils.h \
     $$PWD/appmainwindow.h \
     $$PWD/basetreeview.h \
     $$PWD/elfreader.h \

src/libs/utils/utils.qbs

@@ -220,6 +220,8 @@ Project {
             "portlist.h",
             "processhandle.cpp",
             "processhandle.h",
+            "processutils.cpp",
+            "processutils.h",
             "progressindicator.cpp",
             "progressindicator.h",
             "projectintropage.cpp",

src/tools/processlauncher/CMakeLists.txt

@@ -11,4 +11,6 @@ add_qtc_executable(qtcreator_processlauncher
     processlauncher-main.cpp
     ${UTILSDIR}/launcherpackets.cpp
     ${UTILSDIR}/launcherpackets.h
+    ${UTILSDIR}/processutils.cpp
+    ${UTILSDIR}/processutils.h
     )

src/tools/processlauncher/launchersockethandler.cpp

@@ -77,11 +77,13 @@ public:
     }
 
     quintptr token() const { return m_token; }
+    ProcessStartHandler *processStartHandler() { return &m_processStartHandler; }
 
 private:
     const quintptr m_token;
     QTimer * const m_stopTimer;
     enum class StopState { Inactive, Terminating, Killing } m_stopState = StopState::Inactive;
+    ProcessStartHandler m_processStartHandler;
 };
 
 LauncherSocketHandler::LauncherSocketHandler(QString serverPath, QObject *parent)
@@ -187,6 +189,7 @@ void LauncherSocketHandler::handleProcessStarted()
     Process *proc = senderProcess();
     ProcessStartedPacket packet(proc->token());
     packet.processId = proc->processId();
+    proc->processStartHandler()->handleProcessStarted(proc);
     sendPacket(packet);
 }
 
@@ -237,10 +240,11 @@ void LauncherSocketHandler::handleStartPacket()
     process->setWorkingDirectory(packet.workingDir);
     process->setProcessChannelMode(packet.channelMode);
     process->setStandardInputFile(packet.standardInputFile);
-    process->start(packet.command, packet.arguments, packet.openMode);
-    const bool shouldCloseWriteChannel = !(packet.openMode & QIODevice::WriteOnly);
-    if (shouldCloseWriteChannel)
-        process->closeWriteChannel();
+    ProcessStartHandler *handler = process->processStartHandler();
+    handler->setProcessMode(packet.processMode);
+    handler->setWriteData(packet.writeData);
+    process->start(packet.command, packet.arguments, handler->openMode());
+    handler->handleProcessStart(process);
 }
 
 void LauncherSocketHandler::handleWritePacket()

src/tools/processlauncher/processlauncher.pro

@@ -12,10 +12,12 @@ INCLUDEPATH += $$UTILS_DIR
 HEADERS += \
     launcherlogging.h \
     launchersockethandler.h \
-    $$UTILS_DIR/launcherpackets.h
+    $$UTILS_DIR/launcherpackets.h \
+    $$UTILS_DIR/processutils.h
 
 SOURCES += \
     launcherlogging.cpp \
     launchersockethandler.cpp \
     processlauncher-main.cpp \
-    $$UTILS_DIR/launcherpackets.cpp
+    $$UTILS_DIR/launcherpackets.cpp \
+    $$UTILS_DIR/processutils.cpp

src/tools/processlauncher/processlauncher.qbs

@@ -23,6 +23,8 @@ QtcTool {
         files: [
             "launcherpackets.cpp",
             "launcherpackets.h",
+            "processutils.cpp",
+            "processutils.h",
         ]
     }
 }