Runtime: Don't leak bridged object when value-to-bridged-object cast fails.

[jckarter]

Fixes a leak when a bridgeable value type is dynamically cast to a class type, and the cast fails, for instance:

  let x: Any = "string"
  x is NSNumber

We would fail to release the bridging object after attempting the cast.

[jckarter]

@swift-ci Please test and merge

[gparker42]

This doesn't look right. It looks like the inner cast inherits DestroyOnFailure from the outside. Should this function be doing precisely what _dynamicCastValueToClassExistentialViaObjCBridgeable() does?

[gparker42]

(This function and _dynamicCastValueToClassExistentialViaObjCBridgeable() look like the only relevant uses of bridgeToObjectiveC, so there aren't any similar bugs elsewhere in the file.)

[jckarter]

@gparker42 We need to unconditionally clean up the result if it isn't consumed as part of the cast result. Since it's returned from the bridgeToObjectiveC method, it gets returned as an independent +1 value, independent of the source's lifetime.

Note that when we recursively call _dynamicCastClass on the bridge object, we also mask all DynamicCastFlags except for the Unconditional flag, so that inner cast should default to doing nothing to the bridge object as well.

[jckarter]

@ddunbar @mxcl @rballard These test failures look familiar?

/Users/buildnode/jenkins/workspace/swift-PR-osx/swiftpm/Tests/Functional/DependencyResolutionTests.swift:52: error: -[FunctionalTestSuite.DependencyResolutionTestCase testIndirectTestsDontBuild] : failed - `swift build -c Debug' failed:

exit(1): /Users/buildnode/jenkins/workspace/swift-PR-osx/buildbot_incremental/swiftpm-macosx-x86_64/debug/swift-build --chdir /tmp/DependencyResolution_External_IgnoreIndirectTests.3KmTxL/app --configuration debug

/Users/buildnode/jenkins/workspace/swift-PR-osx/swiftpm/Tests/Functional/DependencyResolutionTests.swift:52: error: -[FunctionalTestSuite.DependencyResolutionTestCase testIndirectTestsDontBuild] : failed - `swift build -c Release' failed:

exit(1): /Users/buildnode/jenkins/workspace/swift-PR-osx/buildbot_incremental/swiftpm-macosx-x86_64/debug/swift-build --chdir /tmp/DependencyResolution_External_IgnoreIndirectTests.3KmTxL/app --configuration release

Test Case '-[FunctionalTestSuite.DependencyResolutionTestCase testIndirectTestsDontBuild]' failed (2.594 seconds).
Test Case '-[FunctionalTestSuite.DependencyResolutionTestCase testInternalComplex]' started.
Test Case '-[FunctionalTestSuite.DependencyResolutionTestCase testInternalComplex]' passed (1.265 seconds).
Test Case '-[FunctionalTestSuite.DependencyResolutionTestCase testInternalSimple]' started.
Test Case '-[FunctionalTestSuite.DependencyResolutionTestCase testInternalSimple]' passed (0.883 seconds).
Test Suite 'DependencyResolutionTestCase' failed at 2016-04-27 16:38:23.962.
     Executed 5 tests, with 2 failures (0 unexpected) in 9.920 (9.920) seconds

[ddunbar]

@moiseev Is this related to the problem we spoke about?

[moiseev]

@ddunbar looks exactly like the failure I saw. Should be fixed by now.

[jckarter]

@swift-ci Please test and merge

[gparker42]

Ah, I mis-read that flags mask operation. But why does the cleanup in _dynamicCastValueToClassViaObjCBridgeable differ from the cleanup in _dynamicCastValueToClassExistentialViaObjCBridgeable ? It would be nice if they looked the same.

[jckarter]

Fair point. One difference is that _dynamicCastValueToClassViaObjCBridgeable calls down to _dynamicCastUnknownClass, which just does a class isa check and doesn't support the dynamic cast flags, whereas _dynamicCastValueToClassExistentialViaObjCBridgeable calls down to _dynamicCastToExistential, which has to handle value representation changes for existentials so does support take-on-success/destroy-on-failure flags.