Merge pull request #2643 from apple/stdlib-unittest-fail-if-child-crashes-at-exit

[WIP] StdlibUnittest: fail if child crashes at exit

Author: gribozavr

stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb

@@ -645,6 +645,12 @@ func _childProcess() {
 
   while let line = _stdlib_getline() {
     let parts = line._split(separator: ";")
+
+    if parts[0] == _stdlibUnittestStreamPrefix {
+      precondition(parts[1] == "shutdown")
+      return
+    }
+
     let testSuiteName = parts[0]
     let testName = parts[1]
     var testParameter: Int?
@@ -670,7 +676,7 @@ func _childProcess() {
 }
 
 struct _ParentProcess {
-  internal var _pid: pid_t = -1
+  internal var _pid: pid_t? = nil
   internal var _childStdin: _FDOutputStream = _FDOutputStream(fd: -1)
   internal var _childStdout: _FDInputStream = _FDInputStream(fd: -1)
   internal var _childStderr: _FDInputStream = _FDInputStream(fd: -1)
@@ -695,8 +701,8 @@ struct _ParentProcess {
   }
 
   mutating func _waitForChild() -> ProcessTerminationStatus {
-    let status = posixWaitpid(_pid)
-    _pid = -1
+    let status = posixWaitpid(_pid!)
+    _pid = nil
     _childStdin.close()
     _childStdout.close()
     _childStderr.close()
@@ -706,6 +712,49 @@ struct _ParentProcess {
     return status
   }
 
+  internal mutating func _readFromChild(
+    onStdoutLine: (String) -> (done: Bool, Void),
+    onStderrLine: (String) -> (done: Bool, Void)
+  ) {
+    var readfds = _stdlib_fd_set()
+    var writefds = _stdlib_fd_set()
+    var errorfds = _stdlib_fd_set()
+    var done = false
+    while !((_childStdout.isEOF && _childStderr.isEOF) || done) {
+      readfds.zero()
+      errorfds.zero()
+      if !_childStdout.isEOF {
+        readfds.set(_childStdout.fd)
+        errorfds.set(_childStdout.fd)
+      }
+      if !_childStderr.isEOF {
+        readfds.set(_childStderr.fd)
+        errorfds.set(_childStderr.fd)
+      }
+      var ret: CInt
+      repeat {
+        ret = _stdlib_select(&readfds, &writefds, &errorfds, nil)
+      } while ret == -1  &&  errno == EINTR
+      if ret <= 0 {
+        fatalError("select() returned an error")
+      }
+      if readfds.isset(_childStdout.fd) || errorfds.isset(_childStdout.fd) {
+        _childStdout.read()
+        while let line = _childStdout.getline() {
+          (done: done, ()) = onStdoutLine(line)
+        }
+        continue
+      }
+      if readfds.isset(_childStderr.fd) || errorfds.isset(_childStderr.fd) {
+        _childStderr.read()
+        while let line = _childStderr.getline() {
+          (done: done, ()) = onStderrLine(line)
+        }
+        continue
+      }
+    }
+  }
+
   /// Returns the values of the corresponding variables in the child process.
   internal mutating func _runTestInChild(
     _ testSuite: TestSuite,
@@ -714,7 +763,7 @@ struct _ParentProcess {
   ) -> (anyExpectFailed: Bool, seenExpectCrash: Bool,
         status: ProcessTerminationStatus?,
         crashStdout: [String], crashStderr: [String]) {
-    if _pid <= 0 {
+    if _pid == nil {
       _spawnChild()
     }
 
@@ -733,95 +782,66 @@ struct _ParentProcess {
       _childStdin.close()
     }
 
-    var readfds = _stdlib_fd_set()
-    var writefds = _stdlib_fd_set()
-    var errorfds = _stdlib_fd_set()
     var stdoutSeenCrashDelimiter = false
     var stderrSeenCrashDelimiter = false
     var stdoutEnd = false
     var stderrEnd = false
     var capturedCrashStdout: [String] = []
     var capturedCrashStderr: [String] = []
     var anyExpectFailedInChild = false
-    while !((_childStdout.isEOF && _childStderr.isEOF) ||
-      (stdoutEnd && stderrEnd)) {
 
-      readfds.zero()
-      errorfds.zero()
-      if !_childStdout.isEOF {
-        readfds.set(_childStdout.fd)
-        errorfds.set(_childStdout.fd)
-      }
-      if !_childStderr.isEOF {
-        readfds.set(_childStderr.fd)
-        errorfds.set(_childStderr.fd)
-      }
-      var ret: CInt
-      repeat {
-        ret = _stdlib_select(&readfds, &writefds, &errorfds, nil)
-      } while ret == -1  &&  errno == EINTR
-      if ret <= 0 {
-        fatalError("select() returned an error")
-      }
-      if readfds.isset(_childStdout.fd) || errorfds.isset(_childStdout.fd) {
-        _childStdout.read()
-        while var line = _childStdout.getline() {
-          if let index = findSubstring(line, _stdlibUnittestStreamPrefix) {
-            let controlMessage =
-                line[index..<line.endIndex]._split(separator: ";")
-            switch controlMessage[1] {
-            case "expectCrash":
-              stdoutSeenCrashDelimiter = true
-              anyExpectFailedInChild = controlMessage[2] == "true"
-            case "end":
-              stdoutEnd = true
-              anyExpectFailedInChild = controlMessage[2] == "true"
-            default:
-              fatalError("unexpected message")
-            }
-            line = line[line.startIndex..<index]
-            if line.isEmpty {
-              continue
-            }
+    func processLine(_ line: String, isStdout: Bool) -> (done: Bool, Void) {
+      var line = line
+      if let index = findSubstring(line, _stdlibUnittestStreamPrefix) {
+        let controlMessage =
+            line[index..<line.endIndex]._split(separator: ";")
+        switch controlMessage[1] {
+        case "expectCrash":
+          if isStdout {
+            stdoutSeenCrashDelimiter = true
+            anyExpectFailedInChild = controlMessage[2] == "true"
+          } else {
+            stderrSeenCrashDelimiter = true
           }
-          if stdoutSeenCrashDelimiter {
-            capturedCrashStdout.append(line)
+        case "end":
+          if isStdout {
+            stdoutEnd = true
+            anyExpectFailedInChild = controlMessage[2] == "true"
+          } else {
+            stderrEnd = true
           }
-          print("stdout>>> \(line)")
+        default:
+          fatalError("unexpected message")
+        }
+        line = line[line.startIndex..<index]
+        if line.isEmpty {
+          return (done: stdoutEnd && stderrEnd, ())
         }
-        continue
       }
-      if readfds.isset(_childStderr.fd) || errorfds.isset(_childStderr.fd) {
-        _childStderr.read()
-        while var line = _childStderr.getline() {
-          if let index = findSubstring(line, _stdlibUnittestStreamPrefix) {
-            let controlMessage =
-                line[index..<line.endIndex]._split(separator: ";")
-            switch controlMessage[1] {
-            case "expectCrash":
-              stderrSeenCrashDelimiter = true
-            case "end":
-              stderrEnd = true
-            default:
-              fatalError("unexpected message")
-            }
-            line = line[line.startIndex..<index]
-            if line.isEmpty {
-              continue
-            }
-          }
-          if stderrSeenCrashDelimiter {
-            capturedCrashStderr.append(line)
-            if findSubstring(line, _crashedPrefix) != nil {
-              line = "OK: saw expected \"\(line.lowercased())\""
-            }
+      if isStdout {
+        if stdoutSeenCrashDelimiter {
+          capturedCrashStdout.append(line)
+        }
+      } else {
+        if stderrSeenCrashDelimiter {
+          capturedCrashStderr.append(line)
+          if findSubstring(line, _crashedPrefix) != nil {
+            line = "OK: saw expected \"\(line.lowercased())\""
           }
-          print("stderr>>> \(line)")
         }
-        continue
       }
+      if isStdout {
+        print("stdout>>> \(line)")
+      } else {
+        print("stderr>>> \(line)")
+      }
+      return (done: stdoutEnd && stderrEnd, ())
     }
 
+    _readFromChild(
+      onStdoutLine: { processLine($0, isStdout: true) },
+      onStderrLine: { processLine($0, isStdout: false) })
+
     // Check if the child has sent us "end" markers for the current test.
     if stdoutEnd && stderrEnd {
       var status: ProcessTerminationStatus? = nil
@@ -851,6 +871,42 @@ struct _ParentProcess {
       capturedCrashStdout, capturedCrashStderr)
   }
 
+  internal mutating func _shutdownChild() -> (failed: Bool, Void) {
+    if _pid == nil {
+      // The child process is not running.  Report that it didn't fail during
+      // shutdown.
+      return (failed: false, ())
+    }
+    print("\(_stdlibUnittestStreamPrefix);shutdown", to: &_childStdin)
+
+    var childCrashed = false
+
+    func processLine(_ line: String, isStdout: Bool) -> (done: Bool, Void) {
+      if isStdout {
+        print("stdout>>> \(line)")
+      } else {
+        if findSubstring(line, _crashedPrefix) != nil {
+          childCrashed = true
+        }
+        print("stderr>>> \(line)")
+      }
+      return (done: false, ())
+    }
+
+    _readFromChild(
+      onStdoutLine: { processLine($0, isStdout: true) },
+      onStderrLine: { processLine($0, isStdout: false) })
+
+    let status = _waitForChild()
+    switch status {
+    case .exit(0):
+      return (failed: childCrashed, ())
+    default:
+      print("Abnormal child process termination: \(status).")
+      return (failed: true, ())
+    }
+  }
+
   internal enum _TestStatus {
     case skip([TestRunPredicate])
     case pass
@@ -1010,6 +1066,11 @@ struct _ParentProcess {
         print("\(testSuite.name): All tests passed")
       }
     }
+    let (failed: failedOnShutdown, ()) = _shutdownChild()
+    if failedOnShutdown {
+      print("The child process failed during shutdown, aborting.")
+      _testSuiteFailedCallback()
+    }
   }
 }
 

validation-test/StdlibUnittest/ChildProcessShutdown/FailIfChildCrashesDuringShutdown.swift

@@ -0,0 +1,35 @@
+// RUN: %target-run-simple-swift 2>&1 | FileCheck %s
+// REQUIRES: executable_test
+
+import StdlibUnittest
+#if os(Linux)
+import Glibc
+#else
+import Darwin
+#endif
+
+_setTestSuiteFailedCallback() { print("abort()") }
+
+//
+// Test that harness aborts when a test crashes if a child process crashes
+// after all tests have finished running.
+//
+
+var TestSuiteChildCrashes = TestSuite("TestSuiteChildCrashes")
+
+TestSuiteChildCrashes.test("passes") {
+  atexit {
+    fatalError("crash at exit")
+  }
+}
+
+// CHECK: [ RUN      ] TestSuiteChildCrashes.passes
+// CHECK: [       OK ] TestSuiteChildCrashes.passes
+// CHECK: TestSuiteChildCrashes: All tests passed
+// CHECK: stderr>>> fatal error: crash at exit:
+// CHECK: stderr>>> CRASHED: SIG
+// CHECK: The child process failed during shutdown, aborting.
+// CHECK: abort()
+
+runAllTests()
+

validation-test/StdlibUnittest/ChildProcessShutdown/FailIfChildExitsDuringShutdown.swift

@@ -0,0 +1,34 @@
+// RUN: %target-run-simple-swift 2>&1 | FileCheck %s
+// REQUIRES: executable_test
+
+import StdlibUnittest
+#if os(Linux)
+import Glibc
+#else
+import Darwin
+#endif
+
+_setTestSuiteFailedCallback() { print("abort()") }
+
+//
+// Test that harness aborts when a test crashes if a child process exits with a
+// non-zero code after all tests have finished running.
+//
+
+var TestSuiteChildExits = TestSuite("TestSuiteChildExits")
+
+TestSuiteChildExits.test("passes") {
+  atexit {
+    _exit(1)
+  }
+}
+
+// CHECK: [ RUN      ] TestSuiteChildExits.passes
+// CHECK: [       OK ] TestSuiteChildExits.passes
+// CHECK: TestSuiteChildExits: All tests passed
+// CHECK: Abnormal child process termination: Exit(1).
+// CHECK: The child process failed during shutdown, aborting.
+// CHECK: abort()
+
+runAllTests()
+

validation-test/StdlibUnittest/ChildProcessShutdown/PassIfChildCrashedDuringTestExecution.swift

@@ -0,0 +1,24 @@
+// RUN: %target-run-simple-swift
+// REQUIRES: executable_test
+
+import StdlibUnittest
+#if os(Linux)
+import Glibc
+#else
+import Darwin
+#endif
+
+//
+// Test that harness correctly handles the case when there is no child process
+// to terminate during shutdown because it crashed during test execution.
+//
+
+var TestSuiteChildCrashes = TestSuite("TestSuiteChildCrashes")
+
+TestSuiteChildCrashes.test("passes") {
+  expectCrashLater()
+  fatalError("crash")
+}
+
+runAllTests()
+

validation-test/StdlibUnittest/CrashingTests.swift

@@ -8,7 +8,7 @@ _setOverrideOSVersion(.osx(major: 10, minor: 9, bugFix: 3))
 _setTestSuiteFailedCallback() { print("abort()") }
 
 //
-// Test that harness aborts when a test crashes
+// Test that harness aborts when a test crashes during a test run.
 //
 
 var TestSuiteCrashes = TestSuite("TestSuiteCrashes")

validation-test/stdlib/Hashing.swift

@@ -144,6 +144,7 @@ HashingTestSuite.test("overridePerExecutionHashSeed/overflow") {
   // Test that we don't use checked arithmetic on the seed.
   _HashingDetail.fixedSeedOverride = UInt64.max
   expectEqual(0x4344_dc3a_239c_3e81, _mixUInt64(0xffff_ffff_ffff_ffff))
+  _HashingDetail.fixedSeedOverride = 0
 }
 
 runAllTests()