Skip to content
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.

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

Fix handling value class with private constructor on proxy #32536

Conversation

T45K
Copy link
Contributor

@T45K T45K commented Mar 26, 2024

Problem

CoroutineUtils and InvocableHandlerMethod will throw exception when value class whose constructor is private is given like

@JvmInline
value class ValueClassWithPrivateConstructor private constructor(val value: String) {
	companion object {
		fun from(value: String) = ValueClassWithPrivateConstructor(value)
	}
}

stacktrace

java.lang.IllegalAccessException: class kotlin.reflect.jvm.internal.calls.CallerImpl$Method cannot access a member of class org.springframework.core.CoroutinesUtilsTests$ValueClassWithPrivateConstructor with modifiers "private static"
kotlin.reflect.full.IllegalCallableAccessException: java.lang.IllegalAccessException: class kotlin.reflect.jvm.internal.calls.CallerImpl$Method cannot access a member of class org.springframework.core.CoroutinesUtilsTests$ValueClassWithPrivateConstructor with modifiers "private static"
	at kotlin.reflect.jvm.internal.KCallableImpl.call(KCallableImpl.kt:280)
	at org.springframework.core.CoroutinesUtils.lambda$invokeSuspendingFunction$3(CoroutinesUtils.java:135)
	at kotlin.coroutines.intrinsics.IntrinsicsKt__IntrinsicsJvmKt$createCoroutineUnintercepted$$inlined$createCoroutineFromSuspendFunction$IntrinsicsKt__IntrinsicsJvmKt$4.invokeSuspend(IntrinsicsJvm.kt:270)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.internal.DispatchedContinuationKt.resumeCancellableWith(DispatchedContinuation.kt:367)
	at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:30)
	at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable$default(Cancellable.kt:25)
	at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:110)
	at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:126)
	at kotlinx.coroutines.reactor.MonoKt.monoInternal$lambda$2(Mono.kt:92)
	at reactor.core.publisher.MonoCreate.subscribe(MonoCreate.java:61)
	at reactor.core.publisher.Mono.subscribe(Mono.java:4568)
	at kotlinx.coroutines.reactor.MonoKt.awaitSingleOrNull(Mono.kt:47)
	at org.springframework.core.CoroutinesUtilsTests$invokeSuspendingFunctionWithValueClassWithPrivateConstructorParameter$1.invokeSuspend(CoroutinesUtilsTests.kt:216)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:280)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at org.springframework.core.CoroutinesUtilsTests.invokeSuspendingFunctionWithValueClassWithPrivateConstructorParameter(CoroutinesUtilsTests.kt:215)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.IllegalAccessException: class kotlin.reflect.jvm.internal.calls.CallerImpl$Method cannot access a member of class org.springframework.core.CoroutinesUtilsTests$ValueClassWithPrivateConstructor with modifiers "private static"
	at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:392)
	at java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:674)
	at java.base/java.lang.reflect.Method.invoke(Method.java:560)
	at kotlin.reflect.jvm.internal.calls.CallerImpl$Method.callMethod(CallerImpl.kt:97)
	at kotlin.reflect.jvm.internal.calls.CallerImpl$Method$Static.call(CallerImpl.kt:106)
	at kotlin.reflect.jvm.internal.calls.ValueClassAwareCaller.call(ValueClassAwareCaller.kt:199)
	at kotlin.reflect.jvm.internal.KCallableImpl.call(KCallableImpl.kt:108)
	... 25 more

Solution

use KCallablesJvm.setAccessible before creating object to avoid the exception

Concern

in this PR, i modified three places. those of them are same. is it better to extract them into a single method?

@T45K T45K changed the title Fix handling value class with private constructor Fix handling value class with private constructor on proxy Mar 26, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 26, 2024
@quaff
Copy link
Contributor

quaff commented Mar 27, 2024

Does it add overhead and is it worthy if it does?

@T45K
Copy link
Contributor Author

T45K commented Mar 27, 2024

hmm, maybe so but i don't know how heavy it is...
let me measure it.
is there any benchmark?

anyway, i think this private constructor issue is regression and should be fixed

@T45K
Copy link
Contributor Author

T45K commented Mar 27, 2024

let me share my initial and rough investigation

i wrote the following test code to measure invoking time by using Kotlin measureNanoTime

@Test
fun measure() {
	val method =
		CoroutinesUtilsTests::class.java.declaredMethods.first { it.name.startsWith("suspendingFunctionWithValueClass") }
	println(measureNanoTime {
		repeat(1_000_000) {
			CoroutinesUtils.invokeSuspendingFunction(method, this, "foo", null)
		}
	})
}

in CoroutinesUtilsTests.

and then, i executed the test five times on each of w/o setAccessible (i.e., comment out KCallablesJvm.setAccessible(valueClassConstructor, true);) and w/ setAccessible

this is the results (unit: nano sec)

1st 2nd 3rd 4th 5th
w/o setAccessible 2,855,264,875 2,724,373,625 1,360,412,625 1,330,190,917 2,604,268,917
w/ setAccessible 1,290,038,333 2,573,693,666 2,516,409,792 2,744,773,333 1,369,282,958

both execution time is around 1.3 sec or 2.7 sec, and there seems no big performance issue

@snicoll snicoll added the theme: kotlin An issue related to Kotlin support label Mar 27, 2024
@sdeleuze sdeleuze self-assigned this Mar 27, 2024
@sdeleuze sdeleuze added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 27, 2024
@sdeleuze sdeleuze added this to the 6.1.6 milestone Mar 27, 2024
@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Mar 27, 2024
sdeleuze pushed a commit to sdeleuze/spring-framework that referenced this pull request Mar 28, 2024
@sdeleuze sdeleuze closed this Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: regression A bug that is also a regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants