New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰Workaround to noescape bug in Xcode 10 beta 1 #537

Merged
merged 2 commits into from Jun 14, 2018

Conversation

Projects
None yet
4 participants
@phatblat
Member

phatblat commented Jun 6, 2018

When using Xcode 10 beta 1, the test suite crashes with the following exception:

Thread 1: closure argument passed as @NoEscape to Objective-C has escaped: file /Users/phatblat/dev/ios/pods/Nimble/Sources/Nimble/DSL+Wait.swift, line 57, column 38

crash at capture.tryBlock

This is a bug in the Swift runtime checker mistakenly identifying the given @noescape closure as escaping. The issue was introduced in #530 and has been logged in
rdar://40857699

This PR applies the workaround suggested by the Apple engineers in the Swift lab at WWDC, which involves removing the noescape attribute from the Objective-C side and annotating the closure as @escaping. This change is temporary as Apple acknowledged this as a bug, but will allow for Xcode 10 to be used for running tests using Nimble without crashing.

@phatblat phatblat requested a review from ikesyo Jun 6, 2018

@ikesyo

This comment has been minimized.

Member

ikesyo commented Jun 7, 2018

I found that the same thing happens on CwlPreconditionTesting which is used in Nimble.

Test Case '-[NimbleTests.ThrowAssertionTest testNegativeMessage]' started.
Fatal error: : file /Users/ikesyo/.ghq/github.com/Quick/Nimble/Tests/NimbleTests/Matchers/ThrowAssertionTest.swift, line 46
2018-06-06 17:27:05.902357-0700 xctest[79915:24107707] Fatal error: : file /Users/ikesyo/.ghq/github.com/Quick/Nimble/Tests/NimbleTests/Matchers/ThrowAssertionTest.swift, line 46
0    Nimble                             0x0000000103ef01c0 catchReturnTypeConverter<A>(_:block:) + 306
1    Nimble                             0x0000000103ef03b0 static NSException.catchException(in:) + 58
2    Nimble                             0x0000000103ec4ef0 closure #4 in catchBadInstruction(in:) + 172
3    Nimble                             0x0000000103ec5030 partial apply for closure #4 in catchBadInstruction(in:) + 35
4    Nimble                             0x0000000103ec5070 thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<MachContext>) -> (@error @owned Error) + 22
5    Nimble                             0x0000000103ec50f0 partial apply for thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<MachContext>) -> (@error @owned Error) + 27
6    libswiftCore.dylib                 0x0000000107337530 withUnsafePointer<A, B>(to:_:) + 12
7    libswiftCore.dylib                 0x00000001074f7ed0 withUnsafeMutablePointer<A, B>(to:_:) + 9
8    Nimble                             0x0000000103ec4170 catchBadInstruction(in:) + 954
9    Nimble                             0x0000000103ef04c0 closure #1 in throwAssertion() + 324
10   Nimble                             0x0000000103efed30 closure #1 in static Predicate.fromDeprecatedClosure(_:) + 120
11   Nimble                             0x0000000103effc60 partial apply for closure #1 in static Predicate.fromDeprecatedClosure(_:) + 38
12   Nimble                             0x0000000103efd8c0 Predicate.satisfies(_:) + 90
13   Nimble                             0x0000000103e7da40 run #1 <A>() in execute<A>(_:_:_:to:description:captureExceptions:) + 520
14   Nimble                             0x0000000103e7e640 closure #2 in execute<A>(_:_:_:to:description:captureExceptions:) + 152
15   Nimble                             0x0000000103e7e850 partial apply for closure #2 in execute<A>(_:_:_:to:description:captureExceptions:) + 83
16   Nimble                             0x0000000103e7e8b0 thunk for @escaping @callee_guaranteed () -> () + 45
17   Nimble                             0x0000000103e6c520 -[NMBExceptionCapture tryBlock:] + 56
18   Nimble                             0x0000000103e7d530 execute<A>(_:_:_:to:description:captureExceptions:) + 876
19   Nimble                             0x0000000103e7f290 Expectation.toNot(_:description:) + 300
20   NimbleTests                        0x0000000103c85420 closure #1 in ThrowAssertionTest.testNegativeMessage() + 358
21   Nimble                             0x0000000103ea1f70 closure #2 in withAssertionHandler(_:closure:) + 34
22   Nimble                             0x0000000103ea2010 partial apply for closure #2 in withAssertionHandler(_:closure:) + 17
23   Nimble                             0x0000000103e7e8b0 thunk for @escaping @callee_guaranteed () -> () + 45
24   Nimble                             0x0000000103e6c520 -[NMBExceptionCapture tryBlock:] + 56
25   Nimble                             0x0000000103ea1b60 withAssertionHandler(_:closure:) + 612
26   NimbleTests                        0x0000000103c1d020 failsWithErrorMessage(_:file:line:preferOriginalSourceLocation:closure:) + 376
27   NimbleTests                        0x0000000103c1eaf0 failsWithErrorMessage(_:file:line:preferOriginalSourceLocation:closure:) + 291
28   NimbleTests                        0x0000000103c85370 ThrowAssertionTest.testNegativeMessage() + 138
29   NimbleTests                        0x0000000103c85650 @objc ThrowAssertionTest.testNegativeMessage() + 36
30   CoreFoundation                     0x00007fff39778bb0 __invoking___ + 140
31   CoreFoundation                     0x00007fff397789d0 -[NSInvocation invoke] + 320
32   XCTest                             0x00000001003761c7 __24-[XCTestCase invokeTest]_block_invoke_2.195 + 65
33   XCTest                             0x00000001003e5008 -[XCTMemoryChecker _assertInvalidObjectsDeallocatedAfterScope:] + 51
34   XCTest                             0x000000010037f11a -[XCTestCase assertInvalidObjectsDeallocatedAfterScope:] + 116
35   XCTest                             0x00000001003760c8 __24-[XCTestCase invokeTest]_block_invoke.189 + 207
36   XCTest                             0x00000001003d4466 +[XCTestCase(Failures) performFailableBlock:shouldInterruptTest:] + 36
37   XCTest                             0x00000001003d439e -[XCTestCase(Failures) _performTurningExceptionsIntoFailuresInterruptAfterHandling:block:] + 54
38   XCTest                             0x0000000100375a7c __24-[XCTestCase invokeTest]_block_invoke + 855
39   XCTest                             0x00000001003d931f -[XCUITestContext performInScope:] + 237
40   XCTest                             0x000000010037596c -[XCTestCase testContextPerformInScope:] + 87
41   XCTest                             0x00000001003759e6 -[XCTestCase invokeTest] + 137
42   XCTest                             0x0000000100377702 __26-[XCTestCase performTest:]_block_invoke_2 + 43
43   XCTest                             0x00000001003d4466 +[XCTestCase(Failures) performFailableBlock:shouldInterruptTest:] + 36
44   XCTest                             0x00000001003d439e -[XCTestCase(Failures) _performTurningExceptionsIntoFailuresInterruptAfterHandling:block:] + 54
45   XCTest                             0x00000001003775ec __26-[XCTestCase performTest:]_block_invoke.334 + 88
46   XCTest                             0x00000001003e1050 +[XCTContext runInContextForTestCase:block:] + 225
47   XCTest                             0x0000000100376ab1 -[XCTestCase performTest:] + 675
48   XCTest                             0x00000001003bb616 -[XCTest runTest] + 57
49   XCTest                             0x0000000100371ba3 __27-[XCTestSuite performTest:]_block_invoke + 365
50   XCTest                             0x00000001003714af -[XCTestSuite _performProtectedSectionForTest:testSection:] + 55
51   XCTest                             0x0000000100371683 -[XCTestSuite performTest:] + 296
52   XCTest                             0x00000001003bb616 -[XCTest runTest] + 57
53   XCTest                             0x0000000100371ba3 __27-[XCTestSuite performTest:]_block_invoke + 365
54   XCTest                             0x00000001003714af -[XCTestSuite _performProtectedSectionForTest:testSection:] + 55
55   XCTest                             0x0000000100371683 -[XCTestSuite performTest:] + 296
56   XCTest                             0x00000001003bb616 -[XCTest runTest] + 57
57   XCTest                             0x0000000100371ba3 __27-[XCTestSuite performTest:]_block_invoke + 365
58   XCTest                             0x00000001003714af -[XCTestSuite _performProtectedSectionForTest:testSection:] + 55
59   XCTest                             0x0000000100371683 -[XCTestSuite performTest:] + 296
60   XCTest                             0x00000001003bb616 -[XCTest runTest] + 57
61   XCTest                             0x00000001003f5a98 __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke + 171
62   XCTest                             0x00000001003f5c20 __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke.78 + 68
63   XCTest                             0x00000001003948f0 -[XCTestObservationCenter _observeTestExecutionForBlock:] + 600
64   XCTest                             0x00000001003f5637 -[XCTTestRunSession runTestsAndReturnError:] + 639
65   XCTest                             0x0000000100357226 -[XCTestDriver runTestsAndReturnError:] + 424
66   XCTest                             0x00000001003dd6bd _XCTestMain + 1493
67   xctest                             0x00000001000022b8 main + 266
68   libdyld.dylib                      0x00007fff61a95014 start + 1
closure argument passed as @noescape to Objective-C has escaped: file /Users/ikesyo/.ghq/github.com/Quick/Nimble/Carthage/Checkouts/CwlCatchException/Sources/CwlCatchException/CwlCatchException.swift, line 28, column 36 
2018-06-06 17:27:06.180223-0700 xctest[79915:24107707] closure argument passed as @noescape to Objective-C has escaped: file /Users/ikesyo/.ghq/github.com/Quick/Nimble/Carthage/Checkouts/CwlCatchException/Sources/CwlCatchException/CwlCatchException.swift, line 28, column 36
/// @see AssertionHandler
public func withAssertionHandler(_ tempAssertionHandler: AssertionHandler, closure: () throws -> Void) {
public func withAssertionHandler(_ tempAssertionHandler: AssertionHandler, closure: @escaping () throws -> Void) {

This comment has been minimized.

@sirlantis

sirlantis Jun 7, 2018

Contributor

Promoting a closure to @escaping will make Swift require explicit self. when accessing properties from within that closure, making this a breaking change.

While this isn't the same as #if XCODE_10, could the following be a compromise?

#if swift(>=4.2)
public func withAssertionHandler(_ tempAssertionHandler: AssertionHandler, closure: @escaping () throws -> Void) {
#else
public func withAssertionHandler(_ tempAssertionHandler: AssertionHandler, closure: () throws -> Void) {
#endif

This comment has been minimized.

@ikesyo

ikesyo Jun 7, 2018

Member

Using withoutActuallyEscaping may be another option.

diff --git a/Sources/Nimble/Adapters/AssertionRecorder.swift b/Sources/Nimble/Adapters/AssertionRecorder.swift
index 1aad65a..0f7adbc 100644
--- a/Sources/Nimble/Adapters/AssertionRecorder.swift
+++ b/Sources/Nimble/Adapters/AssertionRecorder.swift
@@ -46,15 +46,17 @@ public class AssertionRecorder: AssertionHandler {
 /// https://openradar.appspot.com/radar?id=5595735974215680
 ///
 /// @see AssertionHandler
-public func withAssertionHandler(_ tempAssertionHandler: AssertionHandler, closure: @escaping () throws -> Void) {
+public func withAssertionHandler(_ tempAssertionHandler: AssertionHandler, closure: () throws -> Void) {
     let environment = NimbleEnvironment.activeInstance
     let oldRecorder = environment.assertionHandler
     let capturer = NMBExceptionCapture(handler: nil, finally: ({
         environment.assertionHandler = oldRecorder
     }))
     environment.assertionHandler = tempAssertionHandler
-    capturer.tryBlock {
-        try! closure()
+    withoutActuallyEscaping(closure) { escapable in
+        capturer.tryBlock {
+            try! escapable()
+        }
     }
 }
@ikesyo

This comment has been minimized.

Member

ikesyo commented Jun 14, 2018

Looks like the underlying issue has been fixed in apple/swift#17067 and apple/swift#17071 (I hope that the fix will be included in Xcode 10 beta 2).

@ikesyo

ikesyo approved these changes Jun 14, 2018

:shipit:

@ikesyo ikesyo merged commit 69bb161 into master Jun 14, 2018

3 checks passed

Hound No violations found. Woof!
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ikesyo ikesyo deleted the noescape-workaround branch Jun 14, 2018

ikesyo added a commit that referenced this pull request Jun 14, 2018

Merge pull request #537 from Quick/noescape-workaround
馃悰Workaround to noescape bug in Xcode 10 beta 1
@phatblat

This comment has been minimized.

Member

phatblat commented Jun 14, 2018

Thanks for finishing this up @ikesyo! Sorry, I've been slacking lately. Haven't been feeling well.

@ikesyo

This comment has been minimized.

Member

ikesyo commented Jun 20, 2018

#537 (comment)

Looks like the fix is not included in Xcode 10 beta 2 yet unfortunately 馃槶

@morganchen12 morganchen12 referenced this pull request Jun 22, 2018

Closed

Add noescape attribute to unsafeBlock #550

0 of 4 tasks complete
@ikesyo

This comment has been minimized.

Member

ikesyo commented Jul 4, 2018

Xcode 10 beta 3 is out now.

While the release notes says (in "Known Issues in Xcode 10 beta 3 鈥 Apple Swift and LLVM Compilers" section):

When Swift code invokes an Objective-C method that takes a block argument annotated with
__attribute__((noescape)), the Swift runtime may raise an incorrect 鈥渃losure argument passed
as @noescape to Objective-C has escaped鈥 runtime error even when the Objective-C code does not
escape the closure. (40857699)

the issue seems to be fixed. So let's try to revert the changes in this PR.

@zbencz3

This comment has been minimized.

zbencz3 commented Oct 4, 2018

@ikesyo I am still experiencing this problem in XCode 10 GM with the latest release of Nimble. I might be missing something. Any idea?

@ikesyo

This comment has been minimized.

Member

ikesyo commented Oct 4, 2018

I can't reproduce it anymore now with Xcode 10 (10A255) so I'm not sure 馃

@zbencz3

This comment has been minimized.

zbencz3 commented Oct 5, 2018

@ikesyo I have the same XCode version: 10 (10A255).
There is a sample project my colleague created. We both get the crash. Can you please run it to see if you experience the same?
Escaping.zip

crash

@zbencz3 zbencz3 referenced this pull request Oct 8, 2018

Closed

Noescape Nimble crash in XCode 10.0 GM #611

1 of 1 task complete
@ikesyo

This comment has been minimized.

Member

ikesyo commented Oct 16, 2018

#611 and #612 are different from this. This issue is specific for a compiler regression.

@Quick Quick locked as resolved and limited conversation to collaborators Oct 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.