diff --git a/Sources/Foundation/Operation.swift b/Sources/Foundation/Operation.swift index a508d2a743..5c007d34c2 100644 --- a/Sources/Foundation/Operation.swift +++ b/Sources/Foundation/Operation.swift @@ -125,7 +125,7 @@ open class Operation : NSObject { internal var _queue: OperationQueue? { _lock() defer { _unlock() } - return __queue?.takeRetainedValue() + return __queue?.takeUnretainedValue() } internal func _adopt(queue: OperationQueue, schedule: DispatchWorkItem) { @@ -230,7 +230,7 @@ open class Operation : NSObject { let kind: Transition? if keyPath == _NSOperationIsFinished || keyPath == _NSOperationIsFinishedAlternate { kind = .toFinished - } else if keyPath == _NSOperationIsExecuting || keyPath == _NSOperationIsReadyAlternate { + } else if keyPath == _NSOperationIsExecuting || keyPath == _NSOperationIsExecutingAlternate { kind = .toExecuting } else if keyPath == _NSOperationIsReady || keyPath == _NSOperationIsReadyAlternate { kind = .toReady @@ -474,7 +474,7 @@ open class Operation : NSObject { internal func changePriority(_ newPri: Operation.QueuePriority.RawValue) { _lock() - guard let oq = __queue?.takeRetainedValue() else { + guard let oq = __queue?.takeUnretainedValue() else { __priorityValue = newPri _unlock() return @@ -963,6 +963,11 @@ open class OperationQueue : NSObject, ProgressReporting { OperationQueue._currentQueue.set(self) op.start() OperationQueue._currentQueue.clear() + // We've just cleared _currentQueue storage. + // NSThreadSpecific doesn't release stored value on clear. + // This means `self` will leak if we don't release manually. + Unmanaged.passUnretained(self).release() + // unset current tsd if op.isFinished && op._state.rawValue < Operation.__NSOperationState.finishing.rawValue { Operation.observeValue(forKeyPath: _NSOperationIsFinished, ofObject: op) @@ -1086,7 +1091,7 @@ open class OperationQueue : NSObject, ProgressReporting { defer { _unlock() } var op = __firstOperation var cnt = 0 - if let operation = op?.takeUnretainedValue() { + while let operation = op?.takeUnretainedValue() { if !(operation is _BarrierOperation) { cnt += 1 } @@ -1100,7 +1105,7 @@ open class OperationQueue : NSObject, ProgressReporting { defer { _unlock() } var operations = [Operation]() var op = __firstOperation - if let operation = op?.takeUnretainedValue() { + while let operation = op?.takeUnretainedValue() { if includingBarriers || !(operation is _BarrierOperation) { operations.append(operation) } diff --git a/Tests/Foundation/Tests/TestOperationQueue.swift b/Tests/Foundation/Tests/TestOperationQueue.swift index efc09f7e6b..4370e72176 100644 --- a/Tests/Foundation/Tests/TestOperationQueue.swift +++ b/Tests/Foundation/Tests/TestOperationQueue.swift @@ -27,6 +27,18 @@ class TestOperationQueue : XCTestCase { ("test_CurrentQueueWithUnderlyingQueueResetToNil", test_CurrentQueueWithUnderlyingQueueResetToNil), ("test_isSuspended", test_isSuspended), ("test_OperationDependencyCount", test_OperationDependencyCount), + ("test_CancelDependency", test_CancelDependency), + ("test_Deadlock", test_Deadlock), + ("test_CancelOutOfQueue", test_CancelOutOfQueue), + ("test_CrossQueueDependency", test_CrossQueueDependency), + ("test_CancelWhileSuspended", test_CancelWhileSuspended), + ("test_OperationOrder", test_OperationOrder), + ("test_OperationOrder2", test_OperationOrder2), + ("test_WaitUntilFinished", test_WaitUntilFinished), + ("test_OperationWaitUntilFinished", test_OperationWaitUntilFinished), + ("test_CustomOperationReady", test_CustomOperationReady), + ("test_DependencyCycleBreak", test_DependencyCycleBreak), + ("test_Lifecycle", test_Lifecycle), ] } @@ -34,9 +46,24 @@ class TestOperationQueue : XCTestCase { let queue = OperationQueue() let op1 = BlockOperation(block: { Thread.sleep(forTimeInterval: 2) }) queue.addOperation(op1) - XCTAssertTrue(queue.operationCount == 1) + XCTAssertEqual(queue.operationCount, 1) queue.waitUntilAllOperationsAreFinished() - XCTAssertTrue(queue.operationCount == 0) + XCTAssertEqual(queue.operationCount, 0) + + let op2 = BlockOperation(block: { Thread.sleep(forTimeInterval: 0.5) }) + let op3 = BlockOperation(block: { Thread.sleep(forTimeInterval: 0.5) }) + queue.addOperation(op2) + queue.addOperation(op3) + XCTAssertEqual(queue.operationCount, 2) + let operations = queue.operations + XCTAssertEqual(operations.count, 2) + if (operations.count == 2) { + XCTAssertEqual(operations[0], op2) + XCTAssertEqual(operations[1], op3) + } + queue.waitUntilAllOperationsAreFinished() + XCTAssertEqual(queue.operationCount, 0) + XCTAssertEqual(queue.operations.count, 0) } func test_OperationPriorities() { @@ -294,6 +321,357 @@ class TestOperationQueue : XCTestCase { op1.addDependency(op2) XCTAssert(op1.dependencies.count == 1) } + + func test_CancelDependency() { + let expectation = self.expectation(description: "Operation should finish") + + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 1 + + let op1 = BlockOperation() { + XCTAssert(false, "Should not run") + } + let op2 = BlockOperation() { + expectation.fulfill() + } + + op2.addDependency(op1) + op1.cancel() + + queue.addOperation(op1) + queue.addOperation(op2) + + waitForExpectations(timeout: 1) + } + + func test_Deadlock() { + let expectation1 = self.expectation(description: "Operation should finish") + let expectation2 = self.expectation(description: "Operation should finish") + + let op1 = BlockOperation { + expectation1.fulfill() + } + op1.name = "op1" + + let op2 = BlockOperation { + expectation2.fulfill() + } + op2.name = "op2" + + op1.addDependency(op2) + + // Narrow scope to force early release of queue object + _ = { + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 1 + queue.addOperation(op1) + queue.addOperation(op2) + }() + + waitForExpectations(timeout: 1) + Thread.sleep(forTimeInterval: 1) + } + + public func test_CancelOutOfQueue() { + let op = Operation() + op.cancel() + + XCTAssert(op.isCancelled) + XCTAssertFalse(op.isExecuting) + XCTAssertFalse(op.isFinished) + } + + public func test_CrossQueueDependency() { + let queue = OperationQueue() + let queue2 = OperationQueue() + + let expectation1 = self.expectation(description: "Operation should finish") + let expectation2 = self.expectation(description: "Operation should finish") + + let op1 = BlockOperation { + expectation1.fulfill() + } + op1.name = "op1" + + let op2 = BlockOperation { + expectation2.fulfill() + } + op2.name = "op2" + + op1.addDependency(op2) + + queue.addOperation(op1) + queue2.addOperation(op2) + + waitForExpectations(timeout: 1) + } + + public func test_CancelWhileSuspended() { + let queue = OperationQueue() + queue.isSuspended = true + + let op1 = BlockOperation {} + op1.name = "op1" + + let op2 = BlockOperation {} + op2.name = "op2" + + queue.addOperation(op1) + queue.addOperation(op2) + + op1.cancel() + op2.cancel() + + queue.isSuspended = false + queue.waitUntilAllOperationsAreFinished() + + XCTAssert(op1.isCancelled) + XCTAssertFalse(op1.isExecuting) + XCTAssert(op1.isFinished) + XCTAssert(op2.isCancelled) + XCTAssertFalse(op2.isExecuting) + XCTAssert(op2.isFinished) + } + + public func test_OperationOrder() { + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 1 + queue.isSuspended = true + + var array = [Int]() + + let op1 = BlockOperation { + array.append(1) + } + op1.queuePriority = .normal + op1.name = "op1" + + let op2 = BlockOperation { + array.append(2) + } + op2.queuePriority = .normal + op2.name = "op2" + + let op3 = BlockOperation { + array.append(3) + } + op3.queuePriority = .normal + op3.name = "op3" + + let op4 = BlockOperation { + array.append(4) + } + op4.queuePriority = .normal + op4.name = "op4" + + let op5 = BlockOperation { + array.append(5) + } + op5.queuePriority = .normal + op5.name = "op5" + + queue.addOperation(op1) + queue.addOperation(op2) + queue.addOperation(op3) + queue.addOperation(op4) + queue.addOperation(op5) + + queue.isSuspended = false + queue.waitUntilAllOperationsAreFinished() + + XCTAssertEqual(array, [1, 2, 3, 4, 5]) + } + + public func test_OperationOrder2() { + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 1 + queue.isSuspended = true + + var array = [Int]() + + let op1 = BlockOperation { + array.append(1) + } + op1.queuePriority = .veryLow + op1.name = "op1" + + let op2 = BlockOperation { + array.append(2) + } + op2.queuePriority = .low + op2.name = "op2" + + let op3 = BlockOperation { + array.append(3) + } + op3.queuePriority = .normal + op3.name = "op3" + + let op4 = BlockOperation { + array.append(4) + } + op4.queuePriority = .high + op4.name = "op4" + + let op5 = BlockOperation { + array.append(5) + } + op5.queuePriority = .veryHigh + op5.name = "op5" + + queue.addOperation(op1) + queue.addOperation(op2) + queue.addOperation(op3) + queue.addOperation(op4) + queue.addOperation(op5) + + queue.isSuspended = false + queue.waitUntilAllOperationsAreFinished() + + XCTAssertEqual(array, [5, 4, 3, 2, 1]) + } + + func test_WaitUntilFinished() { + let queue1 = OperationQueue() + let queue2 = OperationQueue() + + let op1 = BlockOperation {Thread.sleep(forTimeInterval: 1) } + let op2 = BlockOperation { } + + op2.addDependency(op1) + + queue1.addOperation(op1) + queue2.addOperation(op2) + + queue2.waitUntilAllOperationsAreFinished() + XCTAssertEqual(queue2.operationCount, 0) + } + + func test_OperationWaitUntilFinished() { + let queue1 = OperationQueue() + let op1 = BlockOperation { Thread.sleep(forTimeInterval: 1) } + queue1.addOperation(op1) + op1.waitUntilFinished() + + // Operation is not removed from Queue simultaneously + // with transitioning to "Finished" state. Wait a bit + // to allow OperationQueue to deal with finished op. + Thread.sleep(forTimeInterval: 0.1) + XCTAssertEqual(queue1.operationCount, 0) + } + + func test_CustomOperationReady() { + class CustomOperation: Operation { + + private var _isReady = false + + override var isReady: Bool { + return _isReady + } + + func setIsReady() { + willChangeValue(forKey: "isReady") + _isReady = true + didChangeValue(forKey: "isReady") + } + + } + + let expectation = self.expectation(description: "Operation should finish") + + let queue1 = OperationQueue() + let op1 = CustomOperation() + let op2 = BlockOperation(block: { + expectation.fulfill() + }) + + queue1.addOperation(op1) + queue1.addOperation(op2) + + waitForExpectations(timeout: 1) + + XCTAssertEqual(queue1.operationCount, 1) + op1.setIsReady() + queue1.waitUntilAllOperationsAreFinished() + XCTAssertEqual(queue1.operationCount, 0) + } + + func test_DependencyCycleBreak() { + let op1DidRun = expectation(description: "op1 supposed to be run") + let op2DidRun = expectation(description: "op2 supposed to be run") + let op2Finished = expectation(description: "op2 supposed to be finished") + let op3Cancelled = expectation(description: "op3 supposed to be cancelled") + let op3DidRun = expectation(description: "op3 is not supposed to be run") + op3DidRun.isInverted = true + + var op1: Operation! + var op2: Operation! + var op3: Operation! + + let queue1 = OperationQueue() + op1 = BlockOperation { + op1DidRun.fulfill() + if op2.isFinished { + op2Finished.fulfill() + } + } + op2 = BlockOperation { + op2DidRun.fulfill() + if op3.isCancelled { + op3Cancelled.fulfill() + } + } + op3 = BlockOperation { + op3DidRun.fulfill() + } + + // Create debendency cycle + op1.addDependency(op2) + op2.addDependency(op3) + op3.addDependency(op1) + + queue1.addOperation(op1) + queue1.addOperation(op2) + queue1.addOperation(op3) + + XCTAssertEqual(queue1.operationCount, 3) + + //Break dependency cycle + op3.cancel() + + waitForExpectations(timeout: 1) + } + + func test_Lifecycle() { + let opStarted = expectation(description: "Operation supposed to start") + let opDone = expectation(description: "Operation supposed to be done") + + let op1 = BlockOperation { + Thread.sleep(forTimeInterval: 0.3) + } + let op2 = BlockOperation { + opStarted.fulfill() + Thread.sleep(forTimeInterval: 0.3) + opDone.fulfill() + } + + op1.addDependency(op2) + + weak var weakQueue: OperationQueue? + _ = { + let queue = OperationQueue() + weakQueue = queue + queue.addOperation(op1) + queue.addOperation(op2) + }() + + wait(for: [opStarted], timeout: 1) + op2.cancel() + wait(for: [opDone], timeout: 1) + + Thread.sleep(forTimeInterval: 1) // Let queue to be deallocated + XCTAssertNil(weakQueue, "Queue should be deallocated at this point") + } } class AsyncOperation: Operation {