add WSJT-X code: wrap QProcess to avoid inherited handles causing issues on Windows

Author: IgorChernikov

CMakeLists.txt

@@ -175,6 +175,7 @@ set (wsjt_qt_CXXSRCS
   revision_utils.cpp
   WFPalette.cpp
   Radio.cpp
+  NonInheritingProcess.cpp
   IARURegions.cpp
   Bands.cpp
   Modes.cpp

NonInheritingProcess.cpp

@@ -0,0 +1,116 @@
+#include "NonInheritingProcess.hpp"
+
+#ifdef Q_OS_WIN
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+#ifndef _UNICODE
+#define _UNICODE
+#endif
+#ifdef _WIN32_WINNT
+#undef _WIN32_WINNT
+#endif
+#define _WIN32_WINNT 0x0601
+#include <windows.h>
+#endif
+
+#include <memory>
+#include <functional>
+
+#include "pimpl_impl.hpp"
+
+namespace
+{
+#ifdef Q_OS_WIN
+  struct start_info_deleter
+  {
+    void operator () (STARTUPINFOEXW * si)
+    {
+      if (si->lpAttributeList)
+        {
+          ::DeleteProcThreadAttributeList (si->lpAttributeList);
+        }
+      delete si;
+    }
+  };
+#endif
+}
+
+class NonInheritingProcess::impl
+{
+public:
+#ifdef Q_OS_WIN
+  void extend_CreateProcessArguments (QProcess::CreateProcessArguments * args)
+  {
+    // 
+    //   Here we modify the  CreateProcessArguments structure to use a
+    // STARTUPINFOEX extended  argument to  CreateProcess. In  that we
+    // set up  a list of  handles for the  new process to  inherit. By
+    // doing   this  we   stop  all   inherited  handles   from  being
+    // inherited. Unfortunately UpdateProcThreadAttribute does not let
+    // us set  up an empty handle  list, so we populate  the list with
+    // the three standard stream  handles that QProcess::start has set
+    // up  as Pipes  to do  IPC. Even  though these  Pipe handles  are
+    // created with inheritance disabled, UpdateProcThreadAtribute and
+    // CreateProcess don't seem to mind, which suits us fine.
+    //
+    // Note: that we cannot just clear the inheritHandles flag as that
+    // stops the standard stream  handles being inherited which breaks
+    // our   IPC    using   std(in|out|err).    Only   be    using   a
+    // PROC_THREAD_ATTRIBUTE_HANDLE_LIST attribute  in a STARTUPINFOEX
+    // structure  can  we  avoid  the  all  or  nothing  behaviour  of
+    // CreateProcess /w respect to handle inheritance.
+    // 
+    BOOL fSuccess;
+    SIZE_T size {0};
+    LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = nullptr;
+    ::InitializeProcThreadAttributeList (nullptr, 1, 0, &size);
+    lpAttributeList = reinterpret_cast<LPPROC_THREAD_ATTRIBUTE_LIST> (::HeapAlloc (::GetProcessHeap (), 0, size));
+    fSuccess = !!lpAttributeList;
+    if (fSuccess)
+      {
+        fSuccess = ::InitializeProcThreadAttributeList (lpAttributeList, 1, 0, &size);
+      }
+    if (fSuccess)
+      {
+        // empty list of handles
+        fSuccess = ::UpdateProcThreadAttribute (lpAttributeList, 0,
+                                                PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+                                                &args->startupInfo->hStdInput, 3 * sizeof (HANDLE),
+                                                nullptr, 0);
+      }
+    if (fSuccess)
+      {
+        start_info_.reset (new STARTUPINFOEXW);
+        start_info_->StartupInfo = *args->startupInfo;
+        start_info_->StartupInfo.cb = sizeof (STARTUPINFOEXW);
+        start_info_->lpAttributeList = lpAttributeList;
+        args->startupInfo = reinterpret_cast<Q_STARTUPINFO*> (start_info_.get ());
+        args->flags |= EXTENDED_STARTUPINFO_PRESENT;
+      }
+  }
+
+  using start_info_type = std::unique_ptr<STARTUPINFOEXW, start_info_deleter>;
+  start_info_type start_info_;
+#endif
+};
+
+NonInheritingProcess::NonInheritingProcess (QObject * parent)
+  : QProcess {parent}
+{
+#ifdef Q_OS_WIN
+  using namespace std::placeholders;
+
+  // enable cleanup after process starts or fails to start
+  connect (this, &QProcess::started, [this] {m_->start_info_.reset ();});
+  connect (this, &QProcess::errorOccurred, [this] (QProcess::ProcessError) {m_->start_info_.reset ();});
+  setCreateProcessArgumentsModifier (std::bind (&NonInheritingProcess::impl::extend_CreateProcessArguments, &*m_, _1));
+#endif
+}
+
+NonInheritingProcess::~NonInheritingProcess ()
+{
+}

NonInheritingProcess.hpp

@@ -0,0 +1,35 @@
+#ifndef NON_INHERITING_PROCESS_HPP__
+#define NON_INHERITING_PROCESS_HPP__
+
+#include <QProcess>
+#include "pimpl_h.hpp"
+
+class QObject;
+
+//
+// class NonInheritingProcess - Manage a process without it inheriting
+//                              all inheritable handles
+//
+//   On MS  Windows QProcess  creates sub-processes which  inherit all
+// inheritable  handles, and  handles  on Windows  are inheritable  by
+// default. This can cause the  lifetime of objects to be unexpectedly
+// extended, which in turn can cause unexpected errors. The motivation
+// for this class  was implementing log file rotation  using the Boost
+// log library.  The  current log file's handle gets  inherited by any
+// long  running sub-process  started by  QProcess and  that causes  a
+// sharing  violation  when  attempting  to rename  the  log  file  on
+// rotation, even though  the log library closes the  current log file
+// before trying to rename it.
+//
+class NonInheritingProcess
+  : public QProcess
+{
+public:
+  NonInheritingProcess (QObject * parent = nullptr);
+  ~NonInheritingProcess ();
+
+private:
+  class impl;
+  pimpl<impl> m_;
+};
+#endif

mainwindow.cpp

@@ -738,7 +738,7 @@ MainWindow::MainWindow(bool multiple, QSettings * settings, QSharedMemory *shdme
 
   createStatusBar();
 
-  connect(&proc_jtdxjt9, SIGNAL(readyReadStandardOutput()),this, SLOT(readFromStdout()));
+  connect(&proc_jtdxjt9, &QProcess::readyReadStandardOutput,this, &MainWindow::readFromStdout);
 #if QT_VERSION < QT_VERSION_CHECK (5, 6, 0)
   connect(&proc_jtdxjt9, static_cast<void (QProcess::*) (QProcess::ProcessError)> (&QProcess::error),
           [this] (QProcess::ProcessError error) {
@@ -759,7 +759,7 @@ MainWindow::MainWindow(bool multiple, QSettings * settings, QSharedMemory *shdme
               }
           });
 
-  connect(&p1, SIGNAL(readyReadStandardOutput()),this, SLOT(p1ReadFromStdout()));
+  connect(&p1, &QProcess::readyReadStandardOutput,this, &MainWindow::p1ReadFromStdout);
 #if QT_VERSION < QT_VERSION_CHECK (5, 6, 0)
   connect(&p1, static_cast<void (QProcess::*) (QProcess::ProcessError)> (&QProcess::error),
           [this] (QProcess::ProcessError error) {

mainwindow.h

@@ -24,6 +24,7 @@
 #include <QFileSystemWatcher>
 #include <QClipboard>
 
+#include "NonInheritingProcess.hpp"
 #include "AudioDevice.hpp"
 #include "commons.h"
 #include "Radio.hpp"
@@ -639,9 +640,9 @@ private slots:
 
   QFileSystemWatcher *fsWatcher;
 
-  QProcess proc_jtdxjt9;
-  QProcess p1;
-  QProcess p3;
+  NonInheritingProcess proc_jtdxjt9;
+  NonInheritingProcess p1;
+  NonInheritingProcess p3;
 
   WSPRNet *wsprNet;
   EQSL *Eqsl;