Avoid materializing strong references in potential dangling unmanaged opaque functions

Add test and comments

Author: Azoy

stdlib/public/core/Unmanaged.swift

@@ -34,7 +34,13 @@ public struct Unmanaged<Instance: AnyObject> {
   public static func fromOpaque(
     @_nonEphemeral _ value: UnsafeRawPointer
   ) -> Unmanaged {
-    return Unmanaged(_private: unsafeBitCast(value, to: Instance.self))
+    // NOTE: This function does NOT go through the init(_private:) initializer
+    // because it requires us to materialize a strong reference to 'Instance'.
+    // This materialization is enough to convince the compiler to add
+    // retain/releases which we want to avoid for the opaque pointer functions.
+    // 'Unmanaged<Instance>' is layout compatible with 'UnsafeRawPointer' and
+    // casting to that will not attempt to retain the reference held at 'value'.
+    unsafeBitCast(value, to: Unmanaged<Instance>.self)
   }
 
   /// Unsafely converts an unmanaged class reference to a pointer.
@@ -48,7 +54,12 @@ public struct Unmanaged<Instance: AnyObject> {
   /// - Returns: An opaque pointer to the value of this unmanaged reference.
   @_transparent
   public func toOpaque() -> UnsafeMutableRawPointer {
-    return unsafeBitCast(_value, to: UnsafeMutableRawPointer.self)
+    // NOTE: This function does not attempt to unsafeBitCast '_value' because
+    // that will get a strong reference temporary value who the compiler will
+    // try to retain/release. Use 'self' to avoid this. 'Unmanaged<Instance>' is
+    // layout compatible with 'UnsafeRawPointer' and casting from that will not
+    // attempt to retain the reference held at '_value'.
+    unsafeBitCast(self, to: UnsafeMutableRawPointer.self)
   }
 
   /// Creates an unmanaged reference with an unbalanced retain.

test/stdlib/Unmanaged.swift

@@ -71,4 +71,22 @@ UnmanagedTests.test("Opaque") {
   _fixLifetime(ref)
 }
 
+UnmanagedTests.test("Opaque avoid retain/release") {
+  // The purpose of this test is to ensure that the fromOpaque/toOpaque calls do
+  // NOT attempt to retain/release the class instance at the passed pointer.
+  // Here we're simulating a dangling pointer referencing a class who has
+  // already been released (the allocated pointer points at nothing) and
+  // attempting to create an Unmanaged instance from it and get back the
+  // pointer. This test's expectEqual is kind of bogus, we're just checking that
+  // it doesn't crash.
+
+  let ref = UnsafeMutablePointer<Int>(capacity: 1)
+  let unmanaged = Unmanaged.fromOpaque(UnsafeRawPointer(ref))
+  let ref2 = unmanaged.toOpaque()
+
+  expectEqual(ref, ref2)
+
+  ref.deallocate()
+}
+
 runAllTests()