From 9f44ed353c77a438d6f4ba879b2b388210e2107f Mon Sep 17 00:00:00 2001 From: Alexander Smarus Date: Fri, 27 Nov 2020 19:58:26 +0200 Subject: [PATCH] Fix OperationQueue scheduling logic to prevent disruption of operation lists I believe the original intention was to adust `__nextPriorityOperation`. Doing so for `__nextOperation` has no sense and leads to crosslinking of the main operation list and corresponding priority operation list. OperationQueue in such state would crash or hang while attempting to schedule subsequent operation. --- Sources/Foundation/Operation.swift | 2 +- .../Foundation/Tests/TestOperationQueue.swift | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Sources/Foundation/Operation.swift b/Sources/Foundation/Operation.swift index 5c007d34c2..682dba794d 100644 --- a/Sources/Foundation/Operation.swift +++ b/Sources/Foundation/Operation.swift @@ -993,7 +993,7 @@ open class OperationQueue : NSObject, ProgressReporting { // if the cached state is possibly not valid then the isReady value needs to be re-updated if Operation.__NSOperationState.enqueued == operation._state && operation._fetchCachedIsReady(&retest) { if let previous = prev?.takeUnretainedValue() { - previous.__nextOperation = next + previous.__nextPriorityOperation = next } else { _setFirstPriorityOperation(prio, next) } diff --git a/Tests/Foundation/Tests/TestOperationQueue.swift b/Tests/Foundation/Tests/TestOperationQueue.swift index 4370e72176..e555347380 100644 --- a/Tests/Foundation/Tests/TestOperationQueue.swift +++ b/Tests/Foundation/Tests/TestOperationQueue.swift @@ -34,6 +34,7 @@ class TestOperationQueue : XCTestCase { ("test_CancelWhileSuspended", test_CancelWhileSuspended), ("test_OperationOrder", test_OperationOrder), ("test_OperationOrder2", test_OperationOrder2), + ("test_ExecutionOrder", test_ExecutionOrder), ("test_WaitUntilFinished", test_WaitUntilFinished), ("test_OperationWaitUntilFinished", test_OperationWaitUntilFinished), ("test_CustomOperationReady", test_CustomOperationReady), @@ -531,6 +532,32 @@ class TestOperationQueue : XCTestCase { XCTAssertEqual(array, [5, 4, 3, 2, 1]) } + func test_ExecutionOrder() { + let queue = OperationQueue() + + let didRunOp1 = expectation(description: "Did run first operation") + let didRunOp1Dependency = expectation(description: "Did run first operation dependency") + let didRunOp2 = expectation(description: "Did run second operation") + var didRunOp1DependencyFirst = false + + let op1 = BlockOperation { + didRunOp1.fulfill() + XCTAssertTrue(didRunOp1DependencyFirst, "Dependency should be executed first") + } + let op1Dependency = BlockOperation { + didRunOp1Dependency.fulfill() + didRunOp1DependencyFirst = true + } + op1.addDependency(op1Dependency) + queue.addOperations([op1, op1Dependency], waitUntilFinished: false) + + queue.addOperation { + didRunOp2.fulfill() + } + + waitForExpectations(timeout: 1.0) + } + func test_WaitUntilFinished() { let queue1 = OperationQueue() let queue2 = OperationQueue()