testSafeToExecute*: fix threading issues (#1455)

Motivation: testSafeToExecuteTrue/False accessed an internal property of the event loop without synchronisation. Modifications: Fix the threading issue. Result: Tests with TSan happy.
apple · Mar 18, 2020 · a27a077 · a27a077
1 parent bd4df32
commit a27a077
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 7 deletions.
diff --git a/Sources/NIO/SelectableEventLoop.swift b/Sources/NIO/SelectableEventLoop.swift
@@ -115,8 +115,8 @@ internal final class SelectableEventLoop: EventLoop {
         }
     }
 
-    @usableFromInline
-    internal var _validExternalStateToScheduleTasks: Bool {
+    // access with `externalStateLock` held
+    private var validExternalStateToScheduleTasks: Bool {
         switch self.externalState {
         case .open, .closing:
             return true
@@ -125,6 +125,12 @@ internal final class SelectableEventLoop: EventLoop {
         }
     }
 
+    internal var testsOnly_validExternalStateToScheduleTasks: Bool {
+        return self.externalStateLock.withLock {
+            return self.validExternalStateToScheduleTasks
+        }
+    }
+
     internal init(thread: NIOThread, selector: NIO.Selector<NIORegistration>) {
         self._selector = selector
         self.thread = thread
@@ -261,7 +267,7 @@ internal final class SelectableEventLoop: EventLoop {
             }
         } else {
             self.externalStateLock.withLockVoid {
-                guard self._validExternalStateToScheduleTasks else {
+                guard self.validExternalStateToScheduleTasks else {
                     print("ERROR: Cannot schedule tasks on an EventLoop that has already shut down. " +
                           "This will be upgraded to a forced crash in future SwiftNIO versions.")
                     return

diff --git a/Tests/NIOTests/EventLoopTest.swift b/Tests/NIOTests/EventLoopTest.swift
@@ -1074,15 +1074,15 @@ public final class EventLoopTest : XCTestCase {
             XCTAssertNoThrow(try elg.syncShutdownGracefully())
         }
         let loop = elg.next() as! SelectableEventLoop
-        XCTAssertTrue(loop._validInternalStateToScheduleTasks)
-        XCTAssertTrue(loop._validExternalStateToScheduleTasks)
+        XCTAssertTrue(loop.testsOnly_validExternalStateToScheduleTasks)
+        XCTAssertTrue(loop.testsOnly_validExternalStateToScheduleTasks)
     }
 
     func testSafeToExecuteFalse() {
         let elg = MultiThreadedEventLoopGroup(numberOfThreads: 1)
         let loop = elg.next() as! SelectableEventLoop
         try? elg.syncShutdownGracefully()
-        XCTAssertFalse(loop._validInternalStateToScheduleTasks)
-        XCTAssertFalse(loop._validExternalStateToScheduleTasks)
+        XCTAssertFalse(loop.testsOnly_validExternalStateToScheduleTasks)
+        XCTAssertFalse(loop.testsOnly_validExternalStateToScheduleTasks)
     }
 }