From 6b7e0b3078e7511f358b6aa84af2bf76cd349151 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 28 Jan 2025 11:06:53 +0900 Subject: [PATCH 1/2] Annotate lambdas capturing newer-framework classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit b/391766151 is caused by the combination of three things : • Newer compilers, such as the Kotlin K2 compiler, will use the `invokedynamic` instruction introduced in Java 7. → Before then, lambdas had to be wrapped in synthetic classes, which would be generated by the compiler. The compiler would then generate code that instantiates the synthetic class and pass it to the callee as the lambda. → After then, there is no need for synthetic classes. Instead, the compiler synthesizes a static method that corresponds to the lambda and emits an invokedynamic opcode at that call site. The JVM will then lookup the class dynamically and create a MethodHandle object from which it can substitute an immediate method call at runtime. → The new way means a much smaller footprint (because there are less classes in bytecode) and typically much better call performance after the initial resolution, at the cost of linking the method at the first execution of the lambda. For most modern apps, the trade-off is very advantageous. → In this case, the methods that accept the lambda are of the `runAsShell` family. → Because there are captured local variables of the *WaiterCallback types, the generated static methods take instances of these types. • JUnit uses reflection to enumerate the methods of test classes. It does not take care to only enumerate public methods, and as such will try to resolve all private methods including the types of their arguments. → Note that the `Class` class has a way to retrieve all methods of public visibility including inherited methods, a way to retrieve all non-inherited methods including all visibilities, but not a way to retrieve only public locally-declared methods, so JUnit's implementation is understandable. → In this case, this means JUnit has code that will try to resolve the *WaiterCallback classes. • The test code is compiled against newer SDKs but run with the JUnit test harness on older SDKs which don't have the base classes for the *WaiterCallback classes. The net result is that as JUnit enumerates methods, it finds the private synthetic methods emitted by the newer compilers, which causes the JVM to try and resolve the type of the arguments, which in turn is not possible on older frameworks. The test run promptly crashes with ClassNotFoundException before it can run any test method. Note that this would also happen with newer Java compilers. This patch solves this method by annotating the relevant lambdas with @JvmSerializableLambda, which causes the Kotlin compiler to use the old behavior on just those lambdas. The drawback of this patch is that the @JvmSerializableLambda feature is pretty obscure and it needs to be managed per-lambda. Bug: 391766151 Test: revert aosp/3464183 on top of this, then atest NetworkAgentTest on an S device, then atest CtsNetTestCases to test whether there are other instances of this issue Change-Id: Iacaadddb597c4a5dfdd8c497e6ebc1243ae1e923 --- .../src/android/net/cts/NetworkAgentTest.kt | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 286e08cb9e..4ba41cdef6 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -722,9 +722,15 @@ class NetworkAgentTest { val cv = ConditionVariable() val cpb = PrivilegeWaiterCallback(cv) - tryTest { + // The lambda below is capturing |cpb|, whose type inherits from a class that appeared in + // T. This means the lambda will compile as a private method of this class taking a + // PrivilegeWaiterCallback argument. As JUnit uses reflection to enumerate all methods + // including private methods, this would fail with a link error when running on S-. + // To solve this, make the lambda serializable, which causes the compiler to emit a + // synthetic class instead of a synthetic method. + tryTest @JvmSerializableLambda { val slotIndex = SubscriptionManager.getSlotIndex(subId)!! - runAsShell(READ_PRIVILEGED_PHONE_STATE) { + runAsShell(READ_PRIVILEGED_PHONE_STATE) @JvmSerializableLambda { tm.registerCarrierPrivilegesCallback(slotIndex, { it.run() }, cpb) } // Wait for the callback to be registered @@ -747,8 +753,8 @@ class NetworkAgentTest { carrierConfigRule.cleanUpNow() } assertTrue(cv.block(DEFAULT_TIMEOUT_MS), "Can't change carrier privilege") - } cleanup { - runAsShell(READ_PRIVILEGED_PHONE_STATE) { + } cleanup @JvmSerializableLambda { + runAsShell(READ_PRIVILEGED_PHONE_STATE) @JvmSerializableLambda { tm.unregisterCarrierPrivilegesCallback(cpb) } } @@ -762,9 +768,15 @@ class NetworkAgentTest { val cv = ConditionVariable() val cpb = CarrierServiceChangedWaiterCallback(cv) - tryTest { + // The lambda below is capturing |cpb|, whose type inherits from a class that appeared in + // T. This means the lambda will compile as a private method of this class taking a + // PrivilegeWaiterCallback argument. As JUnit uses reflection to enumerate all methods + // including private methods, this would fail with a link error when running on S-. + // To solve this, make the lambda serializable, which causes the compiler to emit a + // synthetic class instead of a synthetic method. + tryTest @JvmSerializableLambda { val slotIndex = SubscriptionManager.getSlotIndex(subId)!! - runAsShell(READ_PRIVILEGED_PHONE_STATE) { + runAsShell(READ_PRIVILEGED_PHONE_STATE) @JvmSerializableLambda { tm.registerCarrierPrivilegesCallback(slotIndex, { it.run() }, cpb) } // Wait for the callback to be registered @@ -786,8 +798,8 @@ class NetworkAgentTest { } } assertTrue(cv.block(DEFAULT_TIMEOUT_MS), "Can't modify carrier service package") - } cleanup { - runAsShell(READ_PRIVILEGED_PHONE_STATE) { + } cleanup @JvmSerializableLambda { + runAsShell(READ_PRIVILEGED_PHONE_STATE) @JvmSerializableLambda { tm.unregisterCarrierPrivilegesCallback(cpb) } } From cd36064ad46c59214ddd92bd3ffa0f2e90ffc452 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 28 Jan 2025 10:35:15 +0900 Subject: [PATCH 2/2] Revert "Force CtsNetTestCases to compile with Kotlin 1.9" With b/391766151 fixed by the previous patch, this can now be reverted. This reverts commit 4f59de2f85afbeb938646955b44be875e74d07b1. Test: atest NetworkAgentTest on an S device Change-Id: Ie2cb2ca5653a134c0eaa1cac5b0c4b4221f1c0ac --- tests/cts/net/Android.bp | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/cts/net/Android.bp b/tests/cts/net/Android.bp index ce94f87653..1ba581a8d4 100644 --- a/tests/cts/net/Android.bp +++ b/tests/cts/net/Android.bp @@ -24,9 +24,6 @@ java_defaults { "framework-connectivity-test-defaults", ], - // TODO(b/391766151): remove when NetworkAgentTest passes with Kotlin 2. - kotlin_lang_version: "1.9", - // Include both the 32 and 64 bit versions compile_multilib: "both",