Process: kill implementation on windows

rct/Process.h

@@ -114,6 +114,14 @@ class Process
     enum { ReturnCrashed = -1, ReturnUnset = -2, ReturnKilled = -3 };
     int returnCode() const;
 
+    /**
+     * Send the requested signal to the process.
+     *
+     * Windows: Apparently, sending signals is not supported on windows
+     * (although one CAN set signal handlers...), so the windows implementation
+     * kills the process if signal is SIGTERM. For other values, this
+     * method call is ignored.
+     */
     void kill(int signal = SIGTERM);
 
     SignalOnData &readyReadStdOut() { return mReadyReadStdOut; }

rct/Process_Windows.cpp

@@ -13,6 +13,7 @@
 #include "WindowsUnicodeConversion.h"
 
 #include <chrono>
+#include <signal.h>
 
 Process::Process()
     : mMode(Sync), mReturn(ReturnUnset)
@@ -28,6 +29,7 @@ Process::Process()
 
 Process::~Process()
 {
+    kill();
     waitForProcessToFinish();
 
     if(mthStdout.joinable()) mthStdout.join();
@@ -325,23 +327,12 @@ void Process::write(const String &f_data)
 {
     List<String> ret;
 
-    /// RAII class to make sure that the LPTCH object is correctly cleared.
-    class Env_Raii
-    {
-    public:
-        Env_Raii(wchar_t *env) : m_data(env) {}
-        ~Env_Raii() {FreeEnvironmentStringsW(m_data);}
-        wchar_t const *data() const {return m_data;}
-    private:
-        wchar_t * const m_data;
-    };
-
-    Env_Raii envData(GetEnvironmentStringsW());
+    wchar_t * const env = GetEnvironmentStringsW(); //need this pointer later to free
 
-    wchar_t const * readPtr = envData.data();
+    wchar_t const * readPtr = env;
     for(;;)
     {
-        String newEntry = static_cast<const char*>(Utf16To8(readPtr));
+        String newEntry = Utf16To8(readPtr).asStdString();
         readPtr += wcslen(readPtr);  // readPtr now points at entry terminating \0
         readPtr++;   // readPtr now pointers at the beginning of the next entry.
 
@@ -350,9 +341,22 @@ void Process::write(const String &f_data)
         ret.push_back(newEntry);
     }
 
+    FreeEnvironmentStringsW(env);
     return ret;
 }
 
+void Process::kill(int f_signal)
+{
+    if(f_signal != SIGTERM) return;
+    if(mProcess.hProcess == INVALID_HANDLE_VALUE) return;
+    if(mProcessTimeoutStatus == KILLED ||
+       mProcessTimeoutStatus == FINISHED_ON_ITS_OWN) return;
+
+    std::lock_guard<std::mutex> lock(mMutex);
+    TerminateProcess(mProcess.hProcess, ReturnKilled);
+    mProcessTimeoutStatus = KILLED;
+}
+
 int Process::returnCode() const
 {
      std::lock_guard<std::mutex> lock(mMutex);

rct/WindowsUnicodeConversion.h

@@ -26,6 +26,9 @@ class Utf16To8
 
     operator const char        * () const {return m_str.c_str();}
     operator const std::string & () const {return m_str;}
+
+    const std::string &asStdString() const {return m_str;}
+    const char *asCString() const {return m_str.c_str();}
 private:
     std::string m_str;
 };

tests/ProcessTestSuite.cpp

@@ -414,3 +414,29 @@ void ProcessTestSuite::commandLineArgs()
 
     CPPUNIT_ASSERT(data == expected);
 }
+
+void ProcessTestSuite::killWindows()
+{
+    Process p;
+    p.start("ChildProcess");
+
+    realSleep(50);
+    CPPUNIT_ASSERT(!p.isFinished());
+    p.kill();
+    realSleep(50);
+    CPPUNIT_ASSERT(p.isFinished());
+    CPPUNIT_ASSERT(p.returnCode() == Process::ReturnKilled);
+}
+
+void ProcessTestSuite::destructorWindows()
+{
+    std::unique_ptr<Process> p(new Process);
+
+    p->start("ChildProcess");
+    realSleep(50);
+    CPPUNIT_ASSERT(!p->isFinished());
+    p.reset();   //deletes p
+
+    // There should not have been a crash
+    CPPUNIT_ASSERT(true);
+}

tests/ProcessTestSuite.h

@@ -15,6 +15,10 @@ class ProcessTestSuite : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(startNonExistingProgram);
     CPPUNIT_TEST(startUnicodeProgram);
     CPPUNIT_TEST(commandLineArgs);
+#ifdef _WIN32
+    CPPUNIT_TEST(killWindows);
+    CPPUNIT_TEST(destructorWindows);
+#endif
 #ifndef _WIN32
     CPPUNIT_TEST(env);
 #endif
@@ -63,6 +67,15 @@ class ProcessTestSuite : public CPPUNIT_NS::TestFixture
     // Add some command line arguments
     void commandLineArgs();
 
+#ifdef _WIN32
+    // test "kill" on windows -- it can only terminate the child.
+    void killWindows();
+
+    // delete a Process object when the associated child process is still
+    // running -- only works on windows.
+    void destructorWindows();
+#endif
+
 #ifndef _WIN32
     /// Test whether setting the environment for the child process works.
     void env();