Found nasty reflection core bug related to inline classes holding Java types

Support
1

I have a codebase of 10k lines and this bug was very hard to find, after hours of debugging
I finally created a minimal example where this bug is reproducible… Please, I’m in need to resolve this
as soon as possible since I need to go to the production, but this bug is a roadblocking deploy.
If I switch UUID with Int the NullPointerException will not occur.

This code…

@JvmInline
value class Id<T>(val value: UUID = UUID.randomUUID()) {
    override fun toString(): String = this.value.toString()
}

data class Parent(val id: Id<Parent>?)

fun main() {
    val parent = Parent(id = null)
    val kProperty1 = Parent::class.memberProperties.first()

    assert(kProperty1 == Parent::id)
    println("Key '${Parent::id}' has value: '${Parent::id.get(parent)}'")
    println("Key '$kProperty1' has value: ...")
    println(kProperty1.get(parent))
}

Creates this output…

Key 'val org.example.Parent.id: org.example.Id<org.example.Parent>?' has value: 'null'
Key 'val org.example.Parent.id: org.example.Id<org.example.Parent>?' has value: ...
Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.util.UUID.toString()" because "arg0" is null
	at org.example.Id.toString-impl(Main.kt:8)
	at org.example.Id.toString(Main.kt:8)
	at java.base/java.lang.String.valueOf(String.java:4993)
	at java.base/java.io.PrintStream.println(PrintStream.java:1186)
	at org.example.MainKt.main(Main.kt:20)
	at org.example.MainKt.main(Main.kt)

Can this be resolved in other ways that I’m not aware off? The API of the program must stay the same since this is a deploy requirements…

It seems that memberProperties has bad implementation… getting property over memberProperties creates inline object with null value inside and not the null value for the object itself…

2

I definitely can’t help you fix this because I don’t know Kotlin well. I only subscribe to emails for new topics here because I’ve interested in Kotlin lately. But I wanted to jump in and let you know that as far as I know, bugs should always be reported on the YouTrack (https://youtrack.jetbrains.com/issues/KT). I saw some GitHub repos before where maintainers mentioned issues would be migrated there. I think that’s the best ways to get eyes on this bug report.

3

Thank you for repply, yes I did report the bug right away as always, I joust created the post here also since the response from developers is a little bit faster here.

https://youtrack.jetbrains.com/issue/KT-67026/KClass.memberProperties-is-returning-properties-which-are-extracting-invalid-inline-values-from-extractor.

4

I’m trying to understand your code here. It looks like it should be throwing a null pointer because value is null because id is null?

val parent = Parent(id = null)

Not sure if the devs will see it as a bug since it falls in the java interoperability realm where null pointer exceptions are allowed.

The only possibility I see around it is perhaps by implementing your toString method as an extension instead of, or in addition to the method override on Id<T>?. Something like:

Untested

@JvmInline
value class Id<T>(val value: UUID = UUID.randomUUID()) {
   override fun toString(): String = this.value.toString() //perhaps unnecessary? not sure
}

fun <T> Id<T>?.toString() : String  = if (this != null) value.toString() else "null"
5

Parent::class.memberProperties.first() method return the same object instant as Parent::id… That’s why I put assert(Parent::class.memberProperties.first() == Parent::id) assertion… But can you please tell me why if those 2 things are the same they are behaving differently if i’m trying to extract value with get method…

    println("Key '${Parent::id}' has value: '${Parent::id.get(parent)}'")
    println("Key '$kProperty1' has value: ${kProperty1.get(parent)}")

${Parent::id.get(parent)} is returning null value which is totaly correct but kProperty1.get(parent) is returning null pointer exception… and this is wrong because it should return null joust like ${Parent::id.get(parent)} since this is correct behavior…

6

Actually, your code is slightly different in your println’s.

do you get the same exception if you do:

println("Key '$kProperty1' has value: '${kProperty1.get(parent)}'")
7

Actually, this is not true. They not only return different instances, they even return entirely different types of objects:

println(kProperty1::class) // class kotlin.reflect.jvm.internal.KProperty1Impl
println(Parent::id::class) // class TestKt$main$3

Only because == returns true doesn’t meant they are exactly same objects. It returns false if using ===. Also, I wouldn’t generally assume they have to always behave exactly the same, because they are quite different properties - one of them is more “precise”, another is more generic.

But… I think you’re generally right, something is wrong here. We shouldn’t get a boxed Id object with null inside. We should get null instead.

2 Likes
8

I agree that object instances can be different, but underlying behavior should be the same. I would probably conclude that something is wrong with the implementation of memberProperties, it probably
returns objects that are having incomplete implementation inside?

9

Sorry, kinda late to the party.

underlying behavior should be the same

No, not really. == is just call to equals, it can report true for entities of entirely different nature.

More minified example, in terms of produced bytecode. Your printlns and string templates mask the issue, debugger is preferred tool here. Following will throw NPE from line 21 with Kotlin 1.9.23.

/* 1*/ import java.util.UUID
/* 2*/ import kotlin.reflect.KProperty1
/* 3*/ import kotlin.reflect.full.memberProperties
/* 4*/ 
/* 5*/ @JvmInline
/* 6*/ value class Id(val value: UUID = UUID.randomUUID()) {
/* 7*/     override fun toString(): String = value.toString()
/* 8*/ }
/* 9*/ 
/*10*/ data class Parent(val id: Id?)
/*11*/ 
/*12*/ fun main() {
/*13*/     val parent = Parent(id = null)
/*14*/ 
/*15*/     val propertyReference: KProperty1<Parent, Id?> = Parent::id
/*16*/     val valueViaReference /*: Id?*/ = propertyReference.get(parent)
/*17*/     val valueViaReferenceToString = valueViaReference.toString()
/*18*/ 
/*19*/     val propertyReflection: KProperty1<Parent, *> = Parent::class.memberProperties.first()
/*20*/     val valueViaReflection /*: Any?*/ = propertyReflection.get(parent) //as Id?
/*21*/     val valueViaReflectionToString = valueViaReflection.toString()
/*22*/ }

:: is called reference operator
If you take a look at debugger, there will be two different classes:
MainKt$main$propertryReference$1 and KPropertyImpl for propertyReference and propertyReflection respectively.
KProperty1<A, B> is just an interface.

In lines 15,16,19,20 I included same types as inferred by linter for you to see here.
When you use reference, return type (Id?) is known at compile time, so the compiler generates null check.
When you use reflection, compiler has no idea, of what the type is. Notice Id? vs * in lines 15 and 19.

@jonl said about toString()

//perhaps unnecessary? not sure

But it is, I think it is where issue exactly lies, I believe it is indeed a bug in compiler, related to inline value classes.

In line 21 value of valueViaReflection is literally inlined, and our overriden toString() method is called on null, which gives NPE. If Id is not value class, we can see in bytecode that null check are generated. What compiler is should do – not call toString method of instance directly, but use kotlin’s toString extension that will check for null first.

How to mitigate?

API of the program must stay the same

Not sure what your API is, I assume that is definition of Id and Parent.
Uncomment // as Id? in line 20. Compiler will suddenly know that it is nullable thing AND inlined AND it is has it’s own toString to avoid.
Beware, the bug is a bug. :crazy_face:
If you also uncomment /*: Any?*/ in line 20, it’ll break again. Casts seem to be the subject of uncanny optimizations, if (valueViaReflection != null) will probably be most reliable workaround.

In the end of the day reflection seem to work ok, it’s compiler abusing toString of value class.

2 Likes
10

@flistvin Your insight and help is much appreciated, I can see that you know your stuff!
Thank you very very much! :100: