Fix BlockOperation with multiple execution blocks

[lxbndr]

This is rebased version of #2497. Original request seems abandoned, but changes are useful. All credit goes to @dannliu.

Latest Foundation still has this issue: BlockOperation stores first execution block into the internal array along with a second execution block. As the result:

• unexpected value is returned by executionBlocks
• first block will be run twice.

Also @convention(block) specifier in executionBlocks makes dynamic cast crash on return. Looks like it could be safely removed.

[spevans]

@swift-ci test

[spevans]

@lxbndr I ran test_BlockOperationAddExecutionBlock against Catalina native Foundation and the test failed as it looks like OperationQueue can run both blocks concurrently, so sometimes the order was reversed a race-condition meant the array ended up empty.

I modified it as follows and it ran correctly multiple times, I suggest updating the test-case. It also suggests that Darwin may run the blocks differently to Linux.

diff --git a/Tests/Foundation/Tests/TestOperationQueue.swift b/Tests/Foundation/Tests/TestOperationQueue.swift
index 4e55e939..d944f090 100644
--- a/Tests/Foundation/Tests/TestOperationQueue.swift
+++ b/Tests/Foundation/Tests/TestOperationQueue.swift
@@ -702,12 +702,17 @@ class TestOperationQueue : XCTestCase {
     }
 
     func test_BlockOperationAddExecutionBlock() {
-        var msgOperations = [String]()
+        let lock = NSLock()
+        var msgOperations = Set<String>()
         let blockOperation = BlockOperation {
-            msgOperations.append("block1 executed")
+            lock.lock()
+            msgOperations.insert("block1 executed")
+            lock.unlock()
         }
         blockOperation.addExecutionBlock {
-            msgOperations.append("block2 executed")
+            lock.lock()
+            msgOperations.insert("block2 executed")
+            lock.unlock()
         }
         XCTAssert(blockOperation.executionBlocks.count == 2)
         let queue = OperationQueue()

[lxbndr]

@spevans you're right, there is concurrency issue. Didn't noticed that first, as my run of Darwin compatibility tests was lucky to pass.

As we are modifying original request code anyway, would you mind if I change test to use XCTest expectations instead of manual locking?

[spevans]

I dont mind as long as the test runs ok on Darwin. I had to run it quite a few times to see the failures.

[lxbndr]

@spevans done. Thanks for pointing the issue!

[spevans]

@swift-ci test

[spevans]

@swift-ci test

[spevans]

@swift-ci test

[lxbndr]

@spevans should I port it to release branches?

[spevans]

If its a bug fix then yes its fine to port to the release branches.