Skip to content

[pigeon] Fixes support for Kotlin/Java classes that override equals and hashCode for ProxyApis #10039

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
auto-submit merged 15 commits into from
Oct 13, 2025
Merged

[pigeon] Fixes support for Kotlin/Java classes that override equals and hashCode for ProxyApis #10039

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
c828fff
override fix
bparrishMines Sep 19, 2025
3300041
add log and version bump
bparrishMines Sep 19, 2025
dcb38bc
Merge branch 'main' of github.com:flutter/packages into pigeon_overri…
bparrishMines Sep 19, 2025
6d21fdd
better solution for equality
bparrishMines Sep 19, 2025
c369826
Merge branch 'main' of github.com:flutter/packages into pigeon_overri…
bparrishMines Sep 21, 2025
b4fa524
fix pigeonVersion
bparrishMines Sep 21, 2025
3508c81
switch to identity hash code
bparrishMines Sep 21, 2025
906b638
Merge branch 'main' of github.com:flutter/packages into pigeon_overri…
bparrishMines Sep 24, 2025
39db552
use a shared weakreference object
bparrishMines Sep 24, 2025
2e7258f
resort to using the saved hashcode and using the actual instance
bparrishMines Sep 24, 2025
b36e36c
Merge branch 'main' of github.com:flutter/packages into pigeon_overri…
bparrishMines Sep 24, 2025
643f547
add tests for class
bparrishMines Sep 24, 2025
e10b405
another test and more docs
bparrishMines Sep 25, 2025
38efe0f
Merge branch 'main' of github.com:flutter/packages into pigeon_overri…
bparrishMines Sep 25, 2025
f33dd17
create a local method for logging
bparrishMines Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
better solution for equality
  • Loading branch information
@bparrishMines
bparrishMines committed Sep 19, 2025
commit 6d21fdd1e5a001e3767d797e35c3493f216b0b54
33 changes: 27 additions & 6 deletions packages/pigeon/lib/src/kotlin/templates.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,25 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene
fun onFinalize(identifier: Long)
}

private val identifiers = java.util.WeakHashMap<Any, Long>()
// Wraps an instance in a class that overrides the `equals` and `hashCode` methods using identity
// rather than equality.
private class IdentityKey<T : Any> {
private val instance: java.lang.ref.WeakReference<T>

constructor(instance: T) {
this.instance = java.lang.ref.WeakReference<T>(instance)
}

override fun equals(other: Any?): Boolean {
return instance.get() != null && other is IdentityKey<*> && other.instance.get() === instance.get()
}

override fun hashCode(): Int {
return instance.get().hashCode()
}
Comment on lines 68 to 70
Copy link

@gemini-code-assist gemini-code-assist bot Sep 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The hashCode implementation for IdentityKey has a critical bug and a logical inconsistency.

  1. NPE Bug: If the WeakReference has been cleared by the garbage collector, instance.get() will return null. Calling .hashCode() on null will result in a NullPointerException. The equals method correctly handles this case, but hashCode does not.
  2. Logical Inconsistency: The purpose of IdentityKey is to enforce identity-based equality, similar to IdentityHashMap. Therefore, its hash code should also be based on identity. Using the instance's own hashCode() can lead to unnecessary hash collisions if different instances have the same value-based hash code, and it deviates from the principle of identity-based hashing.

The correct approach is to use System.identityHashCode(), which is safe for null inputs (it returns 0) and provides an identity-based hash code.

Suggested change
override fun hashCode(): Int {
return instance.get().hashCode()
}
override fun hashCode(): Int {
return System.identityHashCode(instance.get())
}

}

private val identifiers = java.util.WeakHashMap<IdentityKey<*>, Long>()
private val weakInstances = HashMap<Long, java.lang.ref.WeakReference<Any>>()
private val strongInstances = HashMap<Long, Any>()
private val referenceQueue = java.lang.ref.ReferenceQueue<Any>()
Expand Down Expand Up @@ -112,9 +130,12 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene
*/
fun getIdentifierForStrongReference(instance: Any?): Long? {
logWarningIfFinalizationListenerHasStopped()
val identifier = identifiers[instance]
if (identifier != null && strongInstances[identifier] == null) {
strongInstances[identifier] = instance!!
if (instance == null) {
return null
}
val identifier = identifiers[IdentityKey(instance)]
if (identifier != null) {
strongInstances[identifier] = instance
}
return identifier
}
Expand Down Expand Up @@ -158,7 +179,7 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene
/** Returns whether this manager contains the given `instance`. */
fun containsInstance(instance: Any?): Boolean {
logWarningIfFinalizationListenerHasStopped()
return identifiers.containsKey(instance)
return instance != null && identifiers.containsKey(IdentityKey(instance))
}

/**
Expand Down Expand Up @@ -217,7 +238,7 @@ class ${kotlinInstanceManagerClassName(options)}(private val finalizationListene
"Identifier has already been added: \$identifier"
}
val weakReference = java.lang.ref.WeakReference(instance, referenceQueue)
identifiers[instance] = identifier
identifiers[IdentityKey(instance)] = identifier
weakInstances[identifier] = weakReference
weakReferencesToIdentifiers[weakReference] = identifier
strongInstances[identifier] = instance
Expand Down
35 changes: 29 additions & 6 deletions ...rm_tests/test_plugin/android/src/main/kotlin/com/example/test_plugin/ProxyApiTests.gen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,27 @@ class ProxyApiTestsPigeonInstanceManager(
fun onFinalize(identifier: Long)
}

private val identifiers = java.util.WeakHashMap<Any, Long>()
// Wraps an instance in a class that overrides the `equals` and `hashCode` methods using identity
// rather than equality.
private class IdentityKey<T : Any> {
private val instance: java.lang.ref.WeakReference<T>

constructor(instance: T) {
this.instance = java.lang.ref.WeakReference<T>(instance)
}

override fun equals(other: Any?): Boolean {
return instance.get() != null &&
other is IdentityKey<*> &&
other.instance.get() === instance.get()
}

override fun hashCode(): Int {
return instance.get().hashCode()
}
}

private val identifiers = java.util.WeakHashMap<IdentityKey<*>, Long>()
private val weakInstances = HashMap<Long, java.lang.ref.WeakReference<Any>>()
private val strongInstances = HashMap<Long, Any>()
private val referenceQueue = java.lang.ref.ReferenceQueue<Any>()
Expand Down Expand Up @@ -144,9 +164,12 @@ class ProxyApiTestsPigeonInstanceManager(
*/
fun getIdentifierForStrongReference(instance: Any?): Long? {
logWarningIfFinalizationListenerHasStopped()
val identifier = identifiers[instance]
if (identifier != null && strongInstances[identifier] == null) {
strongInstances[identifier] = instance!!
if (instance == null) {
return null
}
val identifier = identifiers[IdentityKey(instance)]
if (identifier != null) {
strongInstances[identifier] = instance
}
return identifier
}
Expand Down Expand Up @@ -192,7 +215,7 @@ class ProxyApiTestsPigeonInstanceManager(
/** Returns whether this manager contains the given `instance`. */
fun containsInstance(instance: Any?): Boolean {
logWarningIfFinalizationListenerHasStopped()
return identifiers.containsKey(instance)
return instance != null && identifiers.containsKey(IdentityKey(instance))
}

/**
Expand Down Expand Up @@ -252,7 +275,7 @@ class ProxyApiTestsPigeonInstanceManager(
"Identifier has already been added: $identifier"
}
val weakReference = java.lang.ref.WeakReference(instance, referenceQueue)
identifiers[instance] = identifier
identifiers[IdentityKey(instance)] = identifier
weakInstances[identifier] = weakReference
weakReferencesToIdentifiers[weakReference] = identifier
strongInstances[identifier] = instance
Expand Down
36 changes: 31 additions & 5 deletions ..._tests/test_plugin/android/src/test/kotlin/com/example/test_plugin/InstanceManagerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,45 @@ class InstanceManagerTest {
}

@Test
fun getIdentifierForStrongReferenceDoesNotReplaceOriginal() {
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 identifier = instanceManager.addHostCreatedInstance(testObject1)
val identifier1 = instanceManager.addHostCreatedInstance(testObject1)
assertFalse(instanceManager.containsInstance(testObject2))
assertNull(instanceManager.getIdentifierForStrongReference(testObject2))

val testObject2 = TestDataClass(testString)
val identifier2 = instanceManager.addHostCreatedInstance(testObject2)
assertTrue(instanceManager.containsInstance(testObject1))
assertTrue(instanceManager.containsInstance(testObject2))
assertEquals(identifier, instanceManager.getIdentifierForStrongReference(testObject2))
assertTrue(testObject1 === instanceManager.remove(identifier))
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))
}

private fun createInstanceManager(): ProxyApiTestsPigeonInstanceManager {
Expand Down