diff --git a/Sources/Foundation/Operation.swift b/Sources/Foundation/Operation.swift index 027bdf7456..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) diff --git a/Tests/Foundation/Tests/TestOperationQueue.swift b/Tests/Foundation/Tests/TestOperationQueue.swift index a5fc4757f9..4370e72176 100644 --- a/Tests/Foundation/Tests/TestOperationQueue.swift +++ b/Tests/Foundation/Tests/TestOperationQueue.swift @@ -28,17 +28,17 @@ class TestOperationQueue : XCTestCase { ("test_isSuspended", test_isSuspended), ("test_OperationDependencyCount", test_OperationDependencyCount), ("test_CancelDependency", test_CancelDependency), - /* ⚠️ */ ("test_Deadlock", testExpectedToFail(test_Deadlock, "Crashes due to overrelease of OperationQueue")), + ("test_Deadlock", test_Deadlock), ("test_CancelOutOfQueue", test_CancelOutOfQueue), - /* ⚠️ */ ("test_CrossQueueDependency", testExpectedToFail(test_CrossQueueDependency, "Crashes due to overrelease of OperationQueue")), + ("test_CrossQueueDependency", test_CrossQueueDependency), ("test_CancelWhileSuspended", test_CancelWhileSuspended), ("test_OperationOrder", test_OperationOrder), ("test_OperationOrder2", test_OperationOrder2), - /* ⚠️ */ ("test_WaitUntilFinished", testExpectedToFail(test_WaitUntilFinished, "Crashes due to overrelease of OperationQueue")), + ("test_WaitUntilFinished", test_WaitUntilFinished), ("test_OperationWaitUntilFinished", test_OperationWaitUntilFinished), - /* ⚠️ */ ("test_CustomOperationReady", testExpectedToFail(test_CustomOperationReady, "Crashes due to overrelease of OperationQueue")), - /* ⚠️ */ ("test_DependencyCycleBreak", testExpectedToFail(test_DependencyCycleBreak, "Crashes due to overrelease of OperationQueue")), - /* ⚠️ */ ("test_Lifecycle", testExpectedToFail(test_Lifecycle, "Crashes due to overrelease of OperationQueue")), + ("test_CustomOperationReady", test_CustomOperationReady), + ("test_DependencyCycleBreak", test_DependencyCycleBreak), + ("test_Lifecycle", test_Lifecycle), ] } @@ -55,7 +55,12 @@ class TestOperationQueue : XCTestCase { queue.addOperation(op2) queue.addOperation(op3) XCTAssertEqual(queue.operationCount, 2) - XCTAssertEqual(queue.operations.count, 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) @@ -547,6 +552,11 @@ class TestOperationQueue : XCTestCase { 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) } @@ -656,7 +666,7 @@ class TestOperationQueue : XCTestCase { }() wait(for: [opStarted], timeout: 1) - op2.cancel() // op2 + op2.cancel() wait(for: [opDone], timeout: 1) Thread.sleep(forTimeInterval: 1) // Let queue to be deallocated