diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md index d36975e5b0f..0bea707a6e1 100644 --- a/packages/pigeon/CHANGELOG.md +++ b/packages/pigeon/CHANGELOG.md @@ -1,5 +1,7 @@ -## NEXT +## 26.0.2 +* [kotlin] Fixes support for classes that override equals and hashCode for ProxyApis. +* [kotlin] Adds error message log when a new Dart proxy instance fails to be created. * Updates minimum supported SDK version to Flutter 3.29/Dart 3.7. ## 26.0.1 diff --git a/packages/pigeon/lib/src/generator_tools.dart b/packages/pigeon/lib/src/generator_tools.dart index 464750e928a..fea24b6ef45 100644 --- a/packages/pigeon/lib/src/generator_tools.dart +++ b/packages/pigeon/lib/src/generator_tools.dart @@ -15,7 +15,7 @@ import 'generator.dart'; /// The current version of pigeon. /// /// This must match the version in pubspec.yaml. -const String pigeonVersion = '26.0.1'; +const String pigeonVersion = '26.0.2'; /// Read all the content from [stdin] to a String. String readStdin() { diff --git a/packages/pigeon/lib/src/kotlin/kotlin_generator.dart b/packages/pigeon/lib/src/kotlin/kotlin_generator.dart index 79801c4239e..7762d1c1de8 100644 --- a/packages/pigeon/lib/src/kotlin/kotlin_generator.dart +++ b/packages/pigeon/lib/src/kotlin/kotlin_generator.dart @@ -1031,6 +1031,15 @@ if (wrapped == null) { }); indent.newln(); + indent.format(''' + fun logNewInstanceFailure(apiName: String, value: Any, exception: Throwable?) { + Log.w( + "${proxyApiCodecName(const InternalKotlinOptions(kotlinOut: ''))}", + "Failed to create new Dart proxy instance of \$apiName: \$value. \$exception" + ) + } + '''); + enumerate(sortedApis, (int index, AstProxyApi api) { final String className = api.kotlinOptions?.fullClassName ?? api.name; @@ -1043,7 +1052,11 @@ if (wrapped == null) { indent.format(''' ${index > 0 ? ' else ' : ''}if (${versionCheck}value is $className) { - registrar.get$hostProxyApiPrefix${api.name}().${classMemberNamePrefix}newInstance(value) { } + registrar.get$hostProxyApiPrefix${api.name}().${classMemberNamePrefix}newInstance(value) { + if (it.isFailure) { + logNewInstanceFailure("${api.name}", value, it.exceptionOrNull()) + } + } }'''); }); indent.newln(); diff --git a/packages/pigeon/lib/src/kotlin/templates.dart b/packages/pigeon/lib/src/kotlin/templates.dart index 725c594a638..731ec59d0af 100644 --- a/packages/pigeon/lib/src/kotlin/templates.dart +++ b/packages/pigeon/lib/src/kotlin/templates.dart @@ -42,11 +42,39 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene fun onFinalize(identifier: Long) } - private val identifiers = java.util.WeakHashMap() - private val weakInstances = HashMap>() + // Extends WeakReference and overrides the `equals` and `hashCode` methods using identity rather + // than equality. + // + // Two `IdentityWeakReference`s are equal if either + // 1: `get()` returns the identical nonnull value for both references. + // 2: `get()` returns null for both references and the references are identical. + class IdentityWeakReference : java.lang.ref.WeakReference { + private val savedHashCode: Int + + constructor(instance: T) : this(instance, null) + + constructor(instance: T, queue: java.lang.ref.ReferenceQueue?) : super(instance, queue) { + savedHashCode = System.identityHashCode(instance) + } + + override fun equals(other: Any?): Boolean { + val instance = get() + if (instance != null) { + return other is IdentityWeakReference<*> && other.get() === instance + } + return other === this + } + + override fun hashCode(): Int { + return savedHashCode + } + } + + private val identifiers = java.util.WeakHashMap, Long>() + private val weakInstances = HashMap>() private val strongInstances = HashMap() private val referenceQueue = java.lang.ref.ReferenceQueue() - private val weakReferencesToIdentifiers = HashMap, Long>() + private val weakReferencesToIdentifiers = HashMap, Long>() private val handler = android.os.Handler(android.os.Looper.getMainLooper()) private val releaseAllFinalizedInstancesRunnable = Runnable { this.releaseAllFinalizedInstances() @@ -112,9 +140,12 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene */ fun getIdentifierForStrongReference(instance: Any?): Long? { logWarningIfFinalizationListenerHasStopped() - val identifier = identifiers[instance] + if (instance == null) { + return null + } + val identifier = identifiers[IdentityWeakReference(instance)] if (identifier != null) { - strongInstances[identifier] = instance!! + strongInstances[identifier] = instance } return identifier } @@ -151,14 +182,14 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene /** Retrieves the instance associated with identifier, if present, otherwise `null`. */ fun getInstance(identifier: Long): T? { logWarningIfFinalizationListenerHasStopped() - val instance = weakInstances[identifier] as java.lang.ref.WeakReference? + val instance = weakInstances[identifier] as IdentityWeakReference? return instance?.get() } /** Returns whether this manager contains the given `instance`. */ fun containsInstance(instance: Any?): Boolean { logWarningIfFinalizationListenerHasStopped() - return identifiers.containsKey(instance) + return instance != null && identifiers.containsKey(IdentityWeakReference(instance)) } /** @@ -199,8 +230,8 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene if (hasFinalizationListenerStopped()) { return } - var reference: java.lang.ref.WeakReference? - while ((referenceQueue.poll() as java.lang.ref.WeakReference?).also { reference = it } != null) { + var reference: IdentityWeakReference? + while ((referenceQueue.poll() as IdentityWeakReference?).also { reference = it } != null) { val identifier = weakReferencesToIdentifiers.remove(reference) if (identifier != null) { weakInstances.remove(identifier) @@ -216,8 +247,8 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene require(!weakInstances.containsKey(identifier)) { "Identifier has already been added: \$identifier" } - val weakReference = java.lang.ref.WeakReference(instance, referenceQueue) - identifiers[instance] = identifier + val weakReference = IdentityWeakReference(instance, referenceQueue) + identifiers[weakReference] = identifier weakInstances[identifier] = weakReference weakReferencesToIdentifiers[weakReference] = identifier strongInstances[identifier] = instance diff --git a/packages/pigeon/platform_tests/test_plugin/android/src/main/kotlin/com/example/test_plugin/ProxyApiTests.gen.kt b/packages/pigeon/platform_tests/test_plugin/android/src/main/kotlin/com/example/test_plugin/ProxyApiTests.gen.kt index 47a98f16d66..409fd62c7cc 100644 --- a/packages/pigeon/platform_tests/test_plugin/android/src/main/kotlin/com/example/test_plugin/ProxyApiTests.gen.kt +++ b/packages/pigeon/platform_tests/test_plugin/android/src/main/kotlin/com/example/test_plugin/ProxyApiTests.gen.kt @@ -75,11 +75,39 @@ class ProxyApiTestsPigeonInstanceManager( fun onFinalize(identifier: Long) } - private val identifiers = java.util.WeakHashMap() - private val weakInstances = HashMap>() + // Extends WeakReference and overrides the `equals` and `hashCode` methods using identity rather + // than equality. + // + // Two `IdentityWeakReference`s are equal if either + // 1: `get()` returns the identical nonnull value for both references. + // 2: `get()` returns null for both references and the references are identical. + class IdentityWeakReference : java.lang.ref.WeakReference { + private val savedHashCode: Int + + constructor(instance: T) : this(instance, null) + + constructor(instance: T, queue: java.lang.ref.ReferenceQueue?) : super(instance, queue) { + savedHashCode = System.identityHashCode(instance) + } + + override fun equals(other: Any?): Boolean { + val instance = get() + if (instance != null) { + return other is IdentityWeakReference<*> && other.get() === instance + } + return other === this + } + + override fun hashCode(): Int { + return savedHashCode + } + } + + private val identifiers = java.util.WeakHashMap, Long>() + private val weakInstances = HashMap>() private val strongInstances = HashMap() private val referenceQueue = java.lang.ref.ReferenceQueue() - private val weakReferencesToIdentifiers = HashMap, Long>() + private val weakReferencesToIdentifiers = HashMap, Long>() private val handler = android.os.Handler(android.os.Looper.getMainLooper()) private val releaseAllFinalizedInstancesRunnable = Runnable { this.releaseAllFinalizedInstances() @@ -144,9 +172,12 @@ class ProxyApiTestsPigeonInstanceManager( */ fun getIdentifierForStrongReference(instance: Any?): Long? { logWarningIfFinalizationListenerHasStopped() - val identifier = identifiers[instance] + if (instance == null) { + return null + } + val identifier = identifiers[IdentityWeakReference(instance)] if (identifier != null) { - strongInstances[identifier] = instance!! + strongInstances[identifier] = instance } return identifier } @@ -185,14 +216,14 @@ class ProxyApiTestsPigeonInstanceManager( /** Retrieves the instance associated with identifier, if present, otherwise `null`. */ fun getInstance(identifier: Long): T? { logWarningIfFinalizationListenerHasStopped() - val instance = weakInstances[identifier] as java.lang.ref.WeakReference? + val instance = weakInstances[identifier] as IdentityWeakReference? return instance?.get() } /** Returns whether this manager contains the given `instance`. */ fun containsInstance(instance: Any?): Boolean { logWarningIfFinalizationListenerHasStopped() - return identifiers.containsKey(instance) + return instance != null && identifiers.containsKey(IdentityWeakReference(instance)) } /** @@ -233,9 +264,8 @@ class ProxyApiTestsPigeonInstanceManager( if (hasFinalizationListenerStopped()) { return } - var reference: java.lang.ref.WeakReference? - while ((referenceQueue.poll() as java.lang.ref.WeakReference?).also { reference = it } != - null) { + var reference: IdentityWeakReference? + while ((referenceQueue.poll() as IdentityWeakReference?).also { reference = it } != null) { val identifier = weakReferencesToIdentifiers.remove(reference) if (identifier != null) { weakInstances.remove(identifier) @@ -251,8 +281,8 @@ class ProxyApiTestsPigeonInstanceManager( require(!weakInstances.containsKey(identifier)) { "Identifier has already been added: $identifier" } - val weakReference = java.lang.ref.WeakReference(instance, referenceQueue) - identifiers[instance] = identifier + val weakReference = IdentityWeakReference(instance, referenceQueue) + identifiers[weakReference] = identifier weakInstances[identifier] = weakReference weakReferencesToIdentifiers[weakReference] = identifier strongInstances[identifier] = instance @@ -460,14 +490,36 @@ private class ProxyApiTestsPigeonProxyApiBaseCodec( return } + fun logNewInstanceFailure(apiName: String, value: Any, exception: Throwable?) { + Log.w( + "PigeonProxyApiBaseCodec", + "Failed to create new Dart proxy instance of $apiName: $value. $exception") + } + if (value is ProxyApiTestClass) { - registrar.getPigeonApiProxyApiTestClass().pigeon_newInstance(value) {} + registrar.getPigeonApiProxyApiTestClass().pigeon_newInstance(value) { + if (it.isFailure) { + logNewInstanceFailure("ProxyApiTestClass", value, it.exceptionOrNull()) + } + } } else if (value is com.example.test_plugin.ProxyApiSuperClass) { - registrar.getPigeonApiProxyApiSuperClass().pigeon_newInstance(value) {} + registrar.getPigeonApiProxyApiSuperClass().pigeon_newInstance(value) { + if (it.isFailure) { + logNewInstanceFailure("ProxyApiSuperClass", value, it.exceptionOrNull()) + } + } } else if (value is ProxyApiInterface) { - registrar.getPigeonApiProxyApiInterface().pigeon_newInstance(value) {} + registrar.getPigeonApiProxyApiInterface().pigeon_newInstance(value) { + if (it.isFailure) { + logNewInstanceFailure("ProxyApiInterface", value, it.exceptionOrNull()) + } + } } else if (android.os.Build.VERSION.SDK_INT >= 25 && value is ClassWithApiRequirement) { - registrar.getPigeonApiClassWithApiRequirement().pigeon_newInstance(value) {} + registrar.getPigeonApiClassWithApiRequirement().pigeon_newInstance(value) { + if (it.isFailure) { + logNewInstanceFailure("ClassWithApiRequirement", value, it.exceptionOrNull()) + } + } } when { diff --git a/packages/pigeon/platform_tests/test_plugin/android/src/test/kotlin/com/example/test_plugin/InstanceManagerTest.kt b/packages/pigeon/platform_tests/test_plugin/android/src/test/kotlin/com/example/test_plugin/InstanceManagerTest.kt index 7c6a4858b88..b92638f1980 100644 --- a/packages/pigeon/platform_tests/test_plugin/android/src/test/kotlin/com/example/test_plugin/InstanceManagerTest.kt +++ b/packages/pigeon/platform_tests/test_plugin/android/src/test/kotlin/com/example/test_plugin/InstanceManagerTest.kt @@ -149,10 +149,97 @@ class InstanceManagerTest { assertFalse(finalizerRan) } + @Test + fun containsInstanceAndGetIdentifierForStrongReferenceUseIdentityComparison() { + val instanceManager: ProxyApiTestsPigeonInstanceManager = createInstanceManager() + instanceManager.stopFinalizationListener() + + // Create two objects that are equal. + val testString = "aString" + val testObject1 = TestDataClass(testString) + val testObject2 = TestDataClass(testString) + assertEquals(testObject1, testObject2) + + val identifier1 = instanceManager.addHostCreatedInstance(testObject1) + assertFalse(instanceManager.containsInstance(testObject2)) + assertNull(instanceManager.getIdentifierForStrongReference(testObject2)) + + val identifier2 = instanceManager.addHostCreatedInstance(testObject2) + assertTrue(instanceManager.containsInstance(testObject1)) + assertTrue(instanceManager.containsInstance(testObject2)) + assertEquals(identifier1, instanceManager.getIdentifierForStrongReference(testObject1)) + assertEquals(identifier2, instanceManager.getIdentifierForStrongReference(testObject2)) + } + + @Test + fun addingTwoDartCreatedInstancesThatAreEqual() { + val instanceManager: ProxyApiTestsPigeonInstanceManager = createInstanceManager() + instanceManager.stopFinalizationListener() + + // Create two objects that are equal. + val testString = "aString" + val testObject1 = TestDataClass(testString) + val testObject2 = TestDataClass(testString) + assertEquals(testObject1, testObject2) + + instanceManager.addDartCreatedInstance(testObject1, 0) + instanceManager.addDartCreatedInstance(testObject2, 1) + + assertEquals(testObject1, instanceManager.getInstance(0)) + assertEquals(testObject2, instanceManager.getInstance(1)) + assertEquals(0L, instanceManager.getIdentifierForStrongReference(testObject1)) + assertEquals(1L, instanceManager.getIdentifierForStrongReference(testObject2)) + } + + @Test + fun identityWeakReferencesAreEqualWithSameInstance() { + val testObject = Any() + + assertEquals( + ProxyApiTestsPigeonInstanceManager.IdentityWeakReference(testObject), + ProxyApiTestsPigeonInstanceManager.IdentityWeakReference(testObject)) + } + + @Test + fun identityWeakReferenceRemainsEqualAfterGetReturnsNull() { + var testObject: Any? = Any() + + val reference = ProxyApiTestsPigeonInstanceManager.IdentityWeakReference(testObject!!) + + // To allow for object to be garbage collected. + @Suppress("UNUSED_VALUE") + testObject = null + Runtime.getRuntime().gc() + + assertNull(reference.get()) + assertEquals(reference, reference) + } + + @Test + fun identityWeakReferencesAreNotEqualAfterGetReturnsNull() { + var testObject1: Any? = Any() + var testObject2: Any? = Any() + + val reference1 = ProxyApiTestsPigeonInstanceManager.IdentityWeakReference(testObject1!!) + val reference2 = ProxyApiTestsPigeonInstanceManager.IdentityWeakReference(testObject2!!) + + // To allow for object to be garbage collected. + @Suppress("UNUSED_VALUE") + testObject1 = null + testObject2 = null + Runtime.getRuntime().gc() + + assertNull(reference1.get()) + assertNull(reference2.get()) + assertFalse(reference1 == reference2) + } + private fun createInstanceManager(): ProxyApiTestsPigeonInstanceManager { return ProxyApiTestsPigeonInstanceManager.create( object : ProxyApiTestsPigeonInstanceManager.PigeonFinalizationListener { override fun onFinalize(identifier: Long) {} }) } + + data class TestDataClass(val value: String) } diff --git a/packages/pigeon/pubspec.yaml b/packages/pigeon/pubspec.yaml index 3197a83dd4e..3e247f4cfad 100644 --- a/packages/pigeon/pubspec.yaml +++ b/packages/pigeon/pubspec.yaml @@ -2,7 +2,7 @@ name: pigeon description: Code generator tool to make communication between Flutter and the host platform type-safe and easier. repository: https://github.com/flutter/packages/tree/main/packages/pigeon issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+pigeon%22 -version: 26.0.1 # This must match the version in lib/src/generator_tools.dart +version: 26.0.2 # This must match the version in lib/src/generator_tools.dart environment: sdk: ^3.7.0