Test Runner: Fix race in watchdog timeouts.

This fixes a TSAN warning that popped up with the standalone test
runner. The watchdog timer actually was never started so timeouts
were not working as intended. It also switches the timeout mode
to call _Exit which skips all the atexit handlers and avoids some
races on teardown. This change also speeds up the TestSuiteTest.

Also a small fix to GetTempDir that was including an extra path
separator on Windows.

Bug: angleproject:3162
Bug: angleproject:5117
Change-Id: I0e7880a08b61bbb6e30c65665d5c0acec2d78db2
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2442381
Reviewed-by: Yuly Novikov <[email protected]>
Commit-Queue: Jamie Madill <[email protected]>

Author: null77

src/tests/test_utils/runner/TestSuite.cpp

@@ -14,7 +14,9 @@
 #include "common/system_utils.h"
 #include "util/Timer.h"
 
+#include <stdlib.h>
 #include <time.h>
+
 #include <fstream>
 #include <unordered_map>
 
@@ -944,7 +946,7 @@ TestSuite::TestSuite(int *argc, char **argv)
         mResultsFile = resultFileName.str();
     }
 
-    if (!mResultsFile.empty() || !mHistogramJsonFile.empty())
+    if (!mBotMode)
     {
         testing::TestEventListeners &listeners = testing::UnitTest::GetInstance()->listeners();
         listeners.Append(new TestEventListener(mResultsFile, mHistogramJsonFile,
@@ -987,6 +989,7 @@ bool TestSuite::parseSingleArg(const char *argument)
 
 void TestSuite::onCrashOrTimeout(TestResultType crashOrTimeout)
 {
+    std::lock_guard<std::mutex> guard(mTestResults.currentTestMutex);
     if (mTestResults.currentTest.valid())
     {
         TestResult &result        = mTestResults.results[mTestResults.currentTest];
@@ -1191,12 +1194,7 @@ int TestSuite::run()
             mTestResults.allDone = true;
         }
 
-        for (int tries = 0; tries < 10; ++tries)
-        {
-            if (!mWatchdogThread.joinable())
-                break;
-            angle::Sleep(100);
-        }
+        mWatchdogThread.join();
         return retVal;
     }
 
@@ -1333,16 +1331,17 @@ void TestSuite::startWatchdog()
                 if (mTestResults.currentTestTimer.getElapsedTime() >
                     static_cast<double>(mTestTimeout))
                 {
-                    onCrashOrTimeout(TestResultType::Timeout);
-                    exit(2);
+                    break;
                 }
 
                 if (mTestResults.allDone)
                     return;
             }
 
-            angle::Sleep(1000);
+            angle::Sleep(500);
         } while (true);
+        onCrashOrTimeout(TestResultType::Timeout);
+        ::_Exit(1);
     };
     mWatchdogThread = std::thread(watchdogMain);
 }

src/tests/test_utils/runner/TestSuite_unittest.cpp

@@ -48,11 +48,11 @@ TEST_F(TestSuiteTest, RunMockTests)
     executablePath += std::string("/") + kTestHelperExecutable + GetExecutableExtension();
 
     constexpr uint32_t kMaxTempDirLen = 100;
-    char tempFileName[kMaxTempDirLen * 2];
-    ASSERT_TRUE(GetTempDir(tempFileName, kMaxTempDirLen));
+    char tempDirName[kMaxTempDirLen * 2];
+    ASSERT_TRUE(GetTempDir(tempDirName, kMaxTempDirLen));
 
     std::stringstream tempFNameStream;
-    tempFNameStream << tempFileName << "/test_temp_" << rand() << ".json";
+    tempFNameStream << tempDirName << GetPathSeparator() << "test_temp_" << rand() << ".json";
     mTempFileName = tempFNameStream.str();
 
     std::string resultsFileName = "--results-file=" + mTempFileName;
@@ -62,7 +62,7 @@ TEST_F(TestSuiteTest, RunMockTests)
                                       "--gtest_filter=MockTestSuiteTest.DISABLED_*",
                                       "--gtest_also_run_disabled_tests",
                                       "--bot-mode",
-                                      "--test-timeout=10",
+                                      "--test-timeout=2",
                                       resultsFileName.c_str()};
 
     ProcessHandle process(args, true, true);
@@ -71,6 +71,9 @@ TEST_F(TestSuiteTest, RunMockTests)
     EXPECT_TRUE(process->finished());
     EXPECT_EQ(process->getStderr(), "");
 
+    // Uncomment this for debugging.
+    // printf("stdout:\n%s\n", process->getStdout().c_str());
+
     TestResults actual;
     ASSERT_TRUE(GetTestResultsFromFile(mTempFileName.c_str(), &actual));
     EXPECT_TRUE(DeleteFile(mTempFileName.c_str()));
@@ -101,7 +104,7 @@ TEST(MockTestSuiteTest, DISABLED_Fail)
 // Trigger a test timeout.
 TEST(MockTestSuiteTest, DISABLED_Timeout)
 {
-    angle::Sleep(30000);
+    angle::Sleep(5000);
 }
 
 // Trigger a test crash.

util/windows/test_utils_win.cpp

@@ -403,6 +403,15 @@ Process *LaunchProcess(const std::vector<const char *> &args,
 bool GetTempDir(char *tempDirOut, uint32_t maxDirNameLen)
 {
     DWORD pathLen = ::GetTempPathA(maxDirNameLen, tempDirOut);
+    // Strip last path character if present.
+    if (pathLen > 0)
+    {
+        size_t lastChar = strlen(tempDirOut) - 1;
+        if (tempDirOut[lastChar] == '\\')
+        {
+            tempDirOut[lastChar] = 0;
+        }
+    }
     return (pathLen < MAX_PATH && pathLen > 0);
 }