Attempt to fix subprocess handling for Linux (#197)

* Attempt to fix subprocess handling for Linux

Our CI builds are sometimes hanging on Travis, and I suspect it might be something to do with how we are waiting for subprocesses to complete. I'm trying to following the manpage for the `wait()` and `waitpid()` calls a bit better here.

* fixup: try to use poll() instead of select()

* fixup: missing header

* fixup

* fixup

* fixup: try to emit test output when tests fail on Travis

3 files changed, 100 insertions, 55 deletions

diff --git a/tools/slang-test/main.cpp b/tools/slang-test/main.cpp
index 50716d2c4..eeb34b657 100644
--- a/tools/slang-test/main.cpp
+++ b/tools/slang-test/main.cpp
@@ -51,6 +51,11 @@ struct Options
     // force generation of baselines for HLSL tests
     bool generateHLSLBaselines = false;
 
+    // Dump expected/actual output on failures, for debugging.
+    // This is especially intended for use in continuous
+    // integration builds.
+    bool dumpOutputOnFailure = true;
+
     // kind of output to generate
     OutputMode outputMode = kOutputMode_Default;
 
@@ -144,6 +149,11 @@ void parseOptions(int* argc, char** argv)
         else if( strcmp(arg, "-appveyor") == 0 )
         {
             options.outputMode = kOutputMode_AppVeyor;
+            options.dumpOutputOnFailure = true;
+        }
+        else if( strcmp(arg, "-travis") == 0 )
+        {
+            options.dumpOutputOnFailure = true;
         }
         else if( strcmp(arg, "-category") == 0 )
         {
@@ -603,6 +613,21 @@ struct TestInput
 
 typedef TestResult (*TestCallback)(TestInput& input);
 
+void maybeDumpOutput(
+    String const& expectedOutput,
+    String const& actualOutput)
+{
+    if (!options.dumpOutputOnFailure)
+        return;
+
+    fprintf(stderr, "ERROR:\n"
+        "EXPECTED{{{\n%s}}}\n"
+        "ACTUAL{{{\n%s}}}\n",
+        expectedOutput.Buffer(),
+        actualOutput.Buffer());
+    fflush(stderr);
+}
+
 TestResult runSimpleTest(TestInput& input)
 {
     // need to execute the stand-alone Slang compiler on the file, and compare its output to what we expect
@@ -660,15 +685,7 @@ TestResult runSimpleTest(TestInput& input)
         String actualOutputPath = outputStem + ".actual";
         Slang::File::WriteAllText(actualOutputPath, actualOutput);
 
-        if (options.outputMode == kOutputMode_AppVeyor)
-        {
-            fprintf(stderr, "ERROR:\n"
-                "EXPECTED{{{\n%s}}}\n"
-                "ACTUAL{{{\n%s}}}\n",
-                expectedOutput.Buffer(),
-                actualOutput.Buffer());
-            fflush(stderr);
-        }
+        maybeDumpOutput(expectedOutput, actualOutput);
     }
 
     return result;
@@ -731,15 +748,7 @@ TestResult runEvalTest(TestInput& input)
         String actualOutputPath = outputStem + ".actual";
         Slang::File::WriteAllText(actualOutputPath, actualOutput);
 
-        if (options.outputMode == kOutputMode_AppVeyor)
-        {
-            fprintf(stderr, "ERROR:\n"
-                "EXPECTED{{{\n%s}}}\n"
-                "ACTUAL{{{\n%s}}}\n",
-                expectedOutput.Buffer(),
-                actualOutput.Buffer());
-            fflush(stderr);
-        }
+        maybeDumpOutput(expectedOutput, actualOutput);
     }
 
     return result;
@@ -808,15 +817,7 @@ TestResult runCrossCompilerTest(TestInput& input)
         String actualOutputPath = outputStem + ".actual";
         Slang::File::WriteAllText(actualOutputPath, actualOutput);
 
-        if (options.outputMode == kOutputMode_AppVeyor)
-        {
-            fprintf(stderr, "ERROR:\n"
-                "EXPECTED{{{\n%s}}}\n"
-                "ACTUAL{{{\n%s}}}\n",
-                expectedOutput.Buffer(),
-                actualOutput.Buffer());
-            fflush(stderr);
-        }
+        maybeDumpOutput(expectedOutput, actualOutput);
     }
 
     return result;
@@ -949,15 +950,7 @@ TestResult runHLSLComparisonTest(TestInput& input)
         String actualOutputPath = outputStem + ".actual";
         Slang::File::WriteAllText(actualOutputPath, actualOutput);
 
-        if (options.outputMode == kOutputMode_AppVeyor)
-        {
-            fprintf(stderr, "ERROR:\n"
-                "EXPECTED{{{\n%s}}}\n"
-                "ACTUAL{{{\n%s}}}\n",
-                expectedOutput.Buffer(),
-                actualOutput.Buffer());
-            fflush(stderr);
-        }
+        maybeDumpOutput(expectedOutput, actualOutput);
     }
 
     return result;
@@ -1048,6 +1041,8 @@ TestResult runGLSLComparisonTest(TestInput& input)
 
     if (actualOutput != expectedOutput)
     {
+        maybeDumpOutput(expectedOutput, actualOutput);
+
         return kTestResult_Fail;
     }
 
@@ -1193,6 +1188,8 @@ TestResult runHLSLRenderComparisonTestImpl(
 
     if (actualOutput != expectedOutput)
     {
+        maybeDumpOutput(expectedOutput, actualOutput);
+
         return kTestResult_Fail;
     }
 
diff --git a/tools/slang-test/os.cpp b/tools/slang-test/os.cpp
index 18f81318d..4d5a49a9f 100644
--- a/tools/slang-test/os.cpp
+++ b/tools/slang-test/os.cpp
@@ -531,7 +531,10 @@ OSError OSProcessSpawner::spawnAndWaitForCompletion()
         return kOSError_OperationFailed;
 
     pid_t childProcessID = fork();
-    if(childProcessID  == 0)
+    if (childProcessID == -1)
+        return kOSError_OperationFailed;
+
+    if(childProcessID == 0)
     {
         // We are the child process.
 
@@ -562,41 +565,71 @@ OSError OSProcessSpawner::spawnAndWaitForCompletion()
         int stdoutFD = stdoutPipe[0];
         int stderrFD = stderrPipe[0];
 
-        int maxFD = stdoutFD > stderrFD ? stdoutFD : stderrFD;
+        pollfd pollInfos[2];
+        nfds_t pollInfoCount = 2;
 
-        fd_set readSet;
-        int result;
+        pollInfos[0].fd = stdoutFD;
+        pollInfos[0].events = POLLIN;
+        pollInfos[0].revents = 0;
+        pollInfos[1].fd = stderrFD;
+        pollInfos[1].events = POLLIN;
+        pollInfos[1].revents = 0;
 
         int remainingCount =  2;
+        int iterations = 0;
         while(remainingCount)
         {
-            FD_ZERO(&readSet);
-            FD_SET(stdoutFD, &readSet);
-            FD_SET(stderrFD, &readSet);
+            // Safeguard against infinite loop:
+            iterations++;
+            if (iterations > 10000)
+            {
+                fprintf(stderr, "poll(): %d iterations\n", iterations);
+                return kOSError_OperationFailed;
+            }
 
-            result = select(maxFD + 1, &readSet, NULL, NULL, NULL);
+            // Set a timeout of ten seconds;
+            // we really shouldn't wait too long...
+            int pollTimeout = 10000;
+            int pollResult = poll(pollInfos, pollInfoCount, pollTimeout);
+            if (pollResult <= 0)
+            {
+                // If there was a signal that got in
+                // the way, then retry...
+                if(pollResult == -1 && errno == EINTR)
+                    continue;
 
-            if(result == -1 || errno == EINTR)
-                continue;
+                // timeout or error...
+                return kOSError_OperationFailed;
+            }
 
             enum { kBufferSize = 1024 };
             char buffer[kBufferSize];
 
-            if(FD_ISSET(stdoutFD, &readSet))
+            if(pollInfos[0].revents)
             {
                 auto count = read(stdoutFD, buffer, kBufferSize);
-                if(count == 0)
+                if (count <= 0)
+                {
+                    // end-of-file
+                    close(stdoutFD);
+                    pollInfos[0].fd = -1;
                     remainingCount--;
+                }
 
                 standardOutput_.append(
                     buffer, buffer + count);
             }
 
-            if(FD_ISSET(stderrFD, &readSet))
+            if(pollInfos[1].revents)
             {
                 auto count = read(stderrFD, buffer, kBufferSize);
-                if(count == 0)
+                if (count <= 0)
+                {
+                    // end-of-file
+                    close(stderrFD);
+                    pollInfos[1].fd = -1;
                     remainingCount--;
+                }
 
                 standardError_.append(
                     buffer, buffer + count);
@@ -604,24 +637,38 @@ OSError OSProcessSpawner::spawnAndWaitForCompletion()
         }
 
         int childStatus = 0;
+        iterations = 0;
         for(;;)
         {
-            pid_t terminatedProcessID = wait(&childStatus);
+            // Safeguard against infinite loop:
+            iterations++;
+            if (iterations > 10000)
+            {
+                fprintf(stderr, "waitpid(): %d iterations\n", iterations);
+                return kOSError_OperationFailed;
+            }
+
+
+            pid_t terminatedProcessID = waitpid(
+                childProcessID,
+                &childStatus,
+                0);
+            if (terminatedProcessID == -1)
+            {
+                return kOSError_OperationFailed;
+            }
+
             if(terminatedProcessID == childProcessID)
             {
                 if(WIFEXITED(childStatus))
                 {
                     resultCode_ = (int)(int8_t)WEXITSTATUS(childStatus);
-
                 }
                 else
                 {
                     resultCode_ = 1;
                 }
 
-                close(stdoutPipe[0]);
-                close(stderrPipe[0]);
-
                 return kOSError_None;
             }
         }
diff --git a/tools/slang-test/os.h b/tools/slang-test/os.h
index cc1fa4065..3466f818c 100644
--- a/tools/slang-test/os.h
+++ b/tools/slang-test/os.h
@@ -21,6 +21,7 @@
 
 #include <dirent.h>
 #include <errno.h>
+#include <poll.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>