From 18edda5c39b146c97279cc96ee8e63e6883c870f Mon Sep 17 00:00:00 2001 From: Kai Oelfke Date: Thu, 27 Oct 2022 19:03:31 +0200 Subject: [PATCH 1/4] Add disabled environment variable --- .swiftpm/xcode/xcshareddata/xcschemes/Semaphore.xcscheme | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/Semaphore.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/Semaphore.xcscheme index ed39087..0a11c2c 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/Semaphore.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/Semaphore.xcscheme @@ -40,8 +40,15 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - shouldUseLaunchSchemeArgsEnv = "YES" + shouldUseLaunchSchemeArgsEnv = "NO" codeCoverageEnabled = "YES"> + + + + Date: Thu, 27 Oct 2022 19:04:20 +0200 Subject: [PATCH 2/4] Add tests for semaphore incrementing when cancelling --- .../SemaphoreTests/AsyncSemaphoreTests.swift | 91 ++++++++++++++++++- 1 file changed, 90 insertions(+), 1 deletion(-) diff --git a/Tests/SemaphoreTests/AsyncSemaphoreTests.swift b/Tests/SemaphoreTests/AsyncSemaphoreTests.swift index cf9665f..5590bb4 100644 --- a/Tests/SemaphoreTests/AsyncSemaphoreTests.swift +++ b/Tests/SemaphoreTests/AsyncSemaphoreTests.swift @@ -3,6 +3,12 @@ import XCTest @testable import Semaphore final class AsyncSemaphoreTests: XCTestCase { + + override func setUp() { + super.setUp() + // Don't continue after completeWithin(nanoseconds:) causes a XCTFail + continueAfterFailure = false + } func testSignalWithoutSuspendedTasks() async { // Check DispatchSemaphore behavior @@ -218,7 +224,43 @@ final class AsyncSemaphoreTests: XCTestCase { sem.signal() wait(for: [ex2], timeout: 0.5) } - + + func test_that_cancellation_before_suspension_increments_the_semaphore_two() async { + await completeWithin(nanoseconds: NSEC_PER_SEC * 2) { + let sem = AsyncSemaphore(value: 1) + let task = Task { + while !Task.isCancelled { + await Task.yield() + } + try await sem.waitUnlessCancelled() + } + task.cancel() + try? await task.value + await sem.wait() + } + } + + func test_that_cancellation_while_suspended_increments_the_semaphore_two() async { + await completeWithin(nanoseconds: NSEC_PER_SEC * 2) { + let sem = AsyncSemaphore(value: 0) + let running = Atomic(false) + let task = Task { + running.mutate { $0 = true } + try await sem.waitUnlessCancelled() + while !Task.isCancelled { + await Task.yield() + } + } + while !running.value { + await Task.yield() + } + task.cancel() + try? await task.value + sem.signal() + await sem.wait() + } + } + // Test that semaphore can limit the number of concurrent executions of // an actor method. func test_semaphore_as_a_resource_limiter_on_actor_method() async { @@ -395,3 +437,50 @@ final class AsyncSemaphoreTests: XCTestCase { } } } + +/// Helper to complete a test within some amount of time or fail. +/// XCTestExpectation don't work, when using LIBDISPATCH_COOPERATIVE_POOL_STRICT=1 as environment variable +/// or e.g. running tests on an iOS simulator as the wait(for:timeout:) blocks the pool. +/// Which means after the await no further async work can execute to fulfill any expectation that wait +/// is waiting for. +func completeWithin(nanoseconds nanosecondsDeadline: UInt64, + file: StaticString = #filePath, + line: UInt = #line, + work: () async throws -> Void) async rethrows { + let checkDeadlineTask = Task { + try await Task.sleep(nanoseconds: nanosecondsDeadline) + try Task.checkCancellation() + XCTFail("Test timed out.", file: file, line: line) + } + try await work() + checkDeadlineTask.cancel() +} + +final class Atomic: @unchecked Sendable { + private var lock = NSRecursiveLock() + private var _value: A + + public init(_ value: A) { + _value = value + } + + public var value: A { + synced { + _value + } + } + + public func mutate(_ transform: (inout A) -> Void) { + synced { + transform(&self._value) + } + } + + private func synced(_ action: () throws -> Result) rethrows -> Result { + lock.lock() + defer { + lock.unlock() + } + return try action() + } +} From 28464a4e75feb701e5716f3a70d8dfe7fe5cf3a8 Mon Sep 17 00:00:00 2001 From: Kai Oelfke Date: Thu, 27 Oct 2022 19:05:07 +0200 Subject: [PATCH 3/4] Increment semaphore for early cancellation case --- Sources/Semaphore/AsyncSemaphore.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Sources/Semaphore/AsyncSemaphore.swift b/Sources/Semaphore/AsyncSemaphore.swift index cedff00..00b1130 100644 --- a/Sources/Semaphore/AsyncSemaphore.swift +++ b/Sources/Semaphore/AsyncSemaphore.swift @@ -161,9 +161,12 @@ public final class AsyncSemaphore: @unchecked Sendable { value -= 1 if value >= 0 { - unlock() + defer { unlock() } // All code paths check for cancellation - try Task.checkCancellation() + if Task.isCancelled { + value += 1 + throw CancellationError() + } return } From 00f7501e37d8015a185899a81632ad74018d8d14 Mon Sep 17 00:00:00 2001 From: Kai Oelfke Date: Thu, 27 Oct 2022 19:25:33 +0200 Subject: [PATCH 4/4] Await task completion before waiting for blocking expectations --- Tests/SemaphoreTests/AsyncSemaphoreTests.swift | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Tests/SemaphoreTests/AsyncSemaphoreTests.swift b/Tests/SemaphoreTests/AsyncSemaphoreTests.swift index 5590bb4..4d2d200 100644 --- a/Tests/SemaphoreTests/AsyncSemaphoreTests.swift +++ b/Tests/SemaphoreTests/AsyncSemaphoreTests.swift @@ -110,7 +110,7 @@ final class AsyncSemaphoreTests: XCTestCase { let ex1 = expectation(description: "wait") ex1.isInverted = true let ex2 = expectation(description: "woken") - Task { + let task = Task { await sem.wait() ex1.fulfill() ex2.fulfill() @@ -121,6 +121,7 @@ final class AsyncSemaphoreTests: XCTestCase { // When a signal occurs, then the suspended task is resumed. sem.signal() + await task.value wait(for: [ex2], timeout: 0.5) } } @@ -140,6 +141,7 @@ final class AsyncSemaphoreTests: XCTestCase { } try await Task.sleep(nanoseconds: 100_000_000) task.cancel() + await task.value wait(for: [ex], timeout: 1) } @@ -163,6 +165,7 @@ final class AsyncSemaphoreTests: XCTestCase { ex.fulfill() } task.cancel() + await task.value wait(for: [ex], timeout: 5) } @@ -179,7 +182,7 @@ final class AsyncSemaphoreTests: XCTestCase { let ex1 = expectation(description: "wait") ex1.isInverted = true let ex2 = expectation(description: "woken") - Task { + let taskTwo = Task { await sem.wait() ex1.fulfill() ex2.fulfill() @@ -190,6 +193,7 @@ final class AsyncSemaphoreTests: XCTestCase { // When a signal occurs, then the suspended task is resumed. sem.signal() + await taskTwo.value wait(for: [ex2], timeout: 0.5) } @@ -211,7 +215,7 @@ final class AsyncSemaphoreTests: XCTestCase { let ex1 = expectation(description: "wait") ex1.isInverted = true let ex2 = expectation(description: "woken") - Task { + let taskTwo = Task { await sem.wait() ex1.fulfill() ex2.fulfill() @@ -222,6 +226,7 @@ final class AsyncSemaphoreTests: XCTestCase { // When a signal occurs, then the suspended task is resumed. sem.signal() + await taskTwo.value wait(for: [ex2], timeout: 0.5) }