Skip to content

Commit 1500c83

Browse files
authored
Fix 'kotlinx.coroutines.debug' split package problem (Kotlin#4247)
* kotlinx-coroutines-debug: Fix split-package with the -core module - `kotlinx.coroutines.debug.internal` owned by coroutines-core - `kotlinx.coroutines.debug` owned by coroutines-debug (Which also reflects the nature of the debug module, which exposes _some_ public API from the .debug.internal package) This requires moving: - AgentPremain -> .debug.internal - ByteBuddyDynamicAttach -> .debug - NoOpProbes.kt -> .debug * kotlinx-coroutines-debug: Soften requirements on bytebuddy & junit As those dependencies are not *required* to be available at runtime - bytebuddy: Is shadowed and packaged into the -debug.jar - junit: Is optional * AgentInstallationType: Mark as @PublishedApi, as we want to keep the API stable ... as it has known usages in products which we do not want to break
1 parent 1fcffbf commit 1500c83

File tree

20 files changed

+141
-21
lines changed

20 files changed

+141
-21
lines changed

integration-testing/.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
.kotlin
2+
kotlin-js-store

integration-testing/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ The tests are the following:
88
* `coreAgentTest` checks that `kotlinx-coroutines-core` can be run as a Java agent.
99
* `debugAgentTest` checks that the coroutine debugger can be run as a Java agent.
1010
* `debugDynamicAgentTest` checks that `kotlinx-coroutines-debug` agent can self-attach dynamically to JVM as a standalone dependency.
11+
* `debugDynamicAgentJpmsTest` checks that `kotlinx-coroutines-debug` agent can self-attach dynamically to JVM as a standalone dependency (with JPMS)
1112
* `smokeTest` builds the multiplatform test project that depends on coroutines.
1213

1314
The `integration-testing` project is expected to be in a subdirectory of the main `kotlinx.coroutines` project.

integration-testing/build.gradle

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ compileTestKotlin {
167167
}
168168

169169
check {
170-
dependsOn([jvmCoreTest, debugDynamicAgentTest, mavenTest, debugAgentTest, coreAgentTest, 'smokeTest:build'])
170+
dependsOn([jvmCoreTest, debugDynamicAgentTest, mavenTest, debugAgentTest, coreAgentTest, ":jpmsTest:check", 'smokeTest:build'])
171171
}
172172
compileKotlin {
173173
kotlinOptions {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
@file:Suppress("PropertyName")
2+
plugins {
3+
kotlin("jvm")
4+
}
5+
6+
val coroutines_version: String by project
7+
8+
repositories {
9+
if (project.properties["build_snapshot_train"]?.toString()?.toBoolean() == true) {
10+
maven("https://maven.pkg.jetbrains.space/kotlin/p/kotlin/dev")
11+
}
12+
mavenLocal()
13+
mavenCentral()
14+
}
15+
16+
java {
17+
modularity.inferModulePath.set(true)
18+
}
19+
20+
kotlin {
21+
jvmToolchain(17)
22+
23+
val test = target.compilations.getByName("test")
24+
target.compilations.create("debugDynamicAgentJpmsTest") {
25+
associateWith(test)
26+
27+
28+
defaultSourceSet.dependencies {
29+
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:$coroutines_version")
30+
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-debug:$coroutines_version")
31+
}
32+
33+
tasks.register<Test>("debugDynamicAgentJpmsTest") {
34+
testClassesDirs = output.classesDirs
35+
classpath = javaSourceSet.runtimeClasspath
36+
}
37+
}
38+
}
39+
40+
tasks.named("check") {
41+
dependsOn(tasks.withType<Test>())
42+
}
43+
44+
dependencies {
45+
testImplementation(kotlin("test-junit"))
46+
}
47+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module debug.dynamic.agent.jpms.test {
2+
requires kotlin.stdlib;
3+
requires kotlinx.coroutines.core;
4+
requires kotlinx.coroutines.debug;
5+
requires junit;
6+
requires kotlin.test;
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
@file:OptIn(ExperimentalCoroutinesApi::class)
2+
3+
import org.junit.*
4+
import kotlinx.coroutines.*
5+
import kotlinx.coroutines.debug.*
6+
import org.junit.Ignore
7+
import org.junit.Test
8+
import java.io.*
9+
import java.lang.IllegalStateException
10+
import kotlin.test.*
11+
12+
class DynamicAttachDebugJpmsTest {
13+
14+
/**
15+
* This test is disabled because:
16+
* Dynamic Attach with JPMS is not yet supported.
17+
*
18+
* Here is the state of experiments:
19+
* When launching this test with additional workarounds like
20+
* ```
21+
* jvmArgs("--add-exports=kotlinx.coroutines.debug/kotlinx.coroutines.repackaged.net.bytebuddy=com.sun.jna")
22+
* jvmArgs("--add-exports=kotlinx.coroutines.debug/kotlinx.coroutines.repackaged.net.bytebuddy.agent=com.sun.jna")
23+
*```
24+
*
25+
* Then we see issues like
26+
*
27+
* ```
28+
* Caused by: java.lang.IllegalStateException: The Byte Buddy agent is not loaded or this method is not called via the system class loader
29+
* at kotlinx.coroutines.debug/kotlinx.coroutines.repackaged.net.bytebuddy.agent.Installer.getInstrumentation(Installer.java:61)
30+
* ... 54 more
31+
* ```
32+
*/
33+
@Ignore("shaded byte-buddy does not work with JPMS")
34+
@Test
35+
fun testAgentDumpsCoroutines() =
36+
DebugProbes.withDebugProbes {
37+
runBlocking {
38+
val baos = ByteArrayOutputStream()
39+
DebugProbes.dumpCoroutines(PrintStream(baos))
40+
// if the agent works, then dumps should contain something,
41+
// at least the fact that this test is running.
42+
Assert.assertTrue(baos.toString().contains("testAgentDumpsCoroutines"))
43+
}
44+
}
45+
46+
@Test
47+
fun testAgentIsNotInstalled() {
48+
assertEquals(false, DebugProbes.isInstalled)
49+
assertFailsWith<IllegalStateException> {
50+
DebugProbes.dumpCoroutines(PrintStream(ByteArrayOutputStream()))
51+
}
52+
}
53+
54+
}

integration-testing/settings.gradle

+1
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,6 @@ pluginManagement {
88
}
99

1010
include 'smokeTest'
11+
include(":jpmsTest")
1112

1213
rootProject.name = "kotlinx-coroutines-integration-testing"

kotlinx-coroutines-core/api/kotlinx-coroutines-core.api

+4
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,10 @@ public final class kotlinx/coroutines/channels/TickerMode : java/lang/Enum {
911911
public static fun values ()[Lkotlinx/coroutines/channels/TickerMode;
912912
}
913913

914+
public final class kotlinx/coroutines/debug/internal/AgentInstallationType {
915+
public static final field INSTANCE Lkotlinx/coroutines/debug/internal/AgentInstallationType;
916+
}
917+
914918
public final class kotlinx/coroutines/debug/internal/DebugCoroutineInfo {
915919
public final fun getContext ()Lkotlin/coroutines/CoroutineContext;
916920
public final fun getCreationStackTrace ()Ljava/util/List;

kotlinx-coroutines-core/build.gradle.kts

+1-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ val allMetadataJar by tasks.getting(Jar::class) { setupManifest(this) }
188188
fun setupManifest(jar: Jar) {
189189
jar.manifest {
190190
attributes(mapOf(
191-
"Premain-Class" to "kotlinx.coroutines.debug.AgentPremain",
191+
"Premain-Class" to "kotlinx.coroutines.debug.internal.AgentPremain",
192192
"Can-Retransform-Classes" to "true",
193193
))
194194
}

kotlinx-coroutines-core/jvm/resources/META-INF/com.android.tools/proguard/coroutines.pro

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
volatile <fields>;
1717
}
1818

19-
# These classes are only required by kotlinx.coroutines.debug.AgentPremain, which is only loaded when
19+
# These classes are only required by kotlinx.coroutines.debug.internal.AgentPremain, which is only loaded when
2020
# kotlinx-coroutines-core is used as a Java agent, so these are not needed in contexts where ProGuard is used.
2121
-dontwarn java.lang.instrument.ClassFileTransformer
2222
-dontwarn sun.misc.SignalHandler

kotlinx-coroutines-core/jvm/resources/META-INF/com.android.tools/r8/coroutines.pro

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
volatile <fields>;
1313
}
1414

15-
# These classes are only required by kotlinx.coroutines.debug.AgentPremain, which is only loaded when
15+
# These classes are only required by kotlinx.coroutines.debug.internal.AgentPremain, which is only loaded when
1616
# kotlinx-coroutines-core is used as a Java agent, so these are not needed in contexts where ProGuard is used.
1717
-dontwarn java.lang.instrument.ClassFileTransformer
1818
-dontwarn sun.misc.SignalHandler

kotlinx-coroutines-core/jvm/resources/META-INF/proguard/coroutines.pro

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
volatile <fields>;
1717
}
1818

19-
# These classes are only required by kotlinx.coroutines.debug.AgentPremain, which is only loaded when
19+
# These classes are only required by kotlinx.coroutines.debug.internal.AgentPremain, which is only loaded when
2020
# kotlinx-coroutines-core is used as a Java agent, so these are not needed in contexts where ProGuard is used.
2121
-dontwarn java.lang.instrument.ClassFileTransformer
2222
-dontwarn sun.misc.SignalHandler

kotlinx-coroutines-core/jvm/src/debug/internal/AgentInstallationType.kt

+7-1
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,16 @@ package kotlinx.coroutines.debug.internal
33
/**
44
* Object used to differentiate between agent installed statically or dynamically.
55
* This is done in a separate object so [DebugProbesImpl] can check for static installation
6-
* without having to depend on [kotlinx.coroutines.debug.AgentPremain], which is not compatible with Android.
6+
* without having to depend on [AgentPremain], which is not compatible with Android.
77
* Otherwise, access to `AgentPremain.isInstalledStatically` triggers the load of its internal `ClassFileTransformer`
88
* that is not available on Android.
9+
*
10+
* The entity (despite being internal) has usages in the following products
11+
* - Fleet (Reflection): FleetDebugProbes
12+
* - Android (Hard Coded, ignored for Leak Detection)
13+
* - IntelliJ (Suppress KotlinInternalInJava): CoroutineDumpState
914
*/
15+
@PublishedApi
1016
internal object AgentInstallationType {
1117
internal var isInstalledStatically = false
1218
}

kotlinx-coroutines-core/jvm/src/debug/AgentPremain.kt renamed to kotlinx-coroutines-core/jvm/src/debug/internal/AgentPremain.kt

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
package kotlinx.coroutines.debug
1+
package kotlinx.coroutines.debug.internal
22

33
import android.annotation.*
4-
import kotlinx.coroutines.debug.internal.*
54
import org.codehaus.mojo.animal_sniffer.*
65
import sun.misc.*
76
import java.lang.instrument.*

kotlinx-coroutines-core/jvm/src/debug/internal/DebugProbesImpl.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ internal object DebugProbesImpl {
5151

5252
@Suppress("UNCHECKED_CAST")
5353
private fun getDynamicAttach(): Function1<Boolean, Unit>? = runCatching {
54-
val clz = Class.forName("kotlinx.coroutines.debug.internal.ByteBuddyDynamicAttach")
54+
val clz = Class.forName("kotlinx.coroutines.debug.ByteBuddyDynamicAttach")
5555
val ctor = clz.constructors[0]
5656
ctor.newInstance() as Function1<Boolean, Unit>
5757
}.getOrNull()

kotlinx-coroutines-core/jvm/src/module-info.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,12 @@
55
requires transitive kotlin.stdlib;
66
requires kotlinx.atomicfu;
77

8-
// these are used by kotlinx.coroutines.debug.AgentPremain
8+
// these are used by kotlinx.coroutines.debug.internal.AgentPremain
99
requires static java.instrument; // contains java.lang.instrument.*
1010
requires static jdk.unsupported; // contains sun.misc.Signal
1111

1212
exports kotlinx.coroutines;
1313
exports kotlinx.coroutines.channels;
14-
exports kotlinx.coroutines.debug;
1514
exports kotlinx.coroutines.debug.internal;
1615
exports kotlinx.coroutines.flow;
1716
exports kotlinx.coroutines.flow.internal;

kotlinx-coroutines-debug/build.gradle.kts

+2-2
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ val shadowJar by tasks.existing(ShadowJar::class) {
6868
manifest {
6969
attributes(
7070
mapOf(
71-
"Premain-Class" to "kotlinx.coroutines.debug.AgentPremain",
71+
"Premain-Class" to "kotlinx.coroutines.debug.internal.AgentPremain",
7272
"Can-Redefine-Classes" to "true",
7373
"Multi-Release" to "true"
7474
)
@@ -104,7 +104,7 @@ kover {
104104
filters {
105105
excludes {
106106
// Never used, safety mechanism
107-
classes("kotlinx.coroutines.debug.internal.NoOpProbesKt")
107+
classes("kotlinx.coroutines.debug.NoOpProbesKt")
108108
}
109109
}
110110
}

kotlinx-coroutines-debug/src/internal/Attach.kt renamed to kotlinx-coroutines-debug/src/Attach.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
@file:Suppress("unused")
2-
package kotlinx.coroutines.debug.internal
2+
package kotlinx.coroutines.debug
33

44
import net.bytebuddy.*
55
import net.bytebuddy.agent.*
@@ -28,7 +28,7 @@ internal class ByteBuddyDynamicAttach : Function1<Boolean, Unit> {
2828

2929
private fun detach() {
3030
val cl = Class.forName("kotlin.coroutines.jvm.internal.DebugProbesKt")
31-
val cl2 = Class.forName("kotlinx.coroutines.debug.internal.NoOpProbesKt")
31+
val cl2 = Class.forName("kotlinx.coroutines.debug.NoOpProbesKt")
3232
ByteBuddy()
3333
.redefine(cl2)
3434
.name(cl.name)

kotlinx-coroutines-debug/src/internal/NoOpProbes.kt renamed to kotlinx-coroutines-debug/src/NoOpProbes.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
@file:Suppress("unused", "UNUSED_PARAMETER")
22

3-
package kotlinx.coroutines.debug.internal
3+
package kotlinx.coroutines.debug
44

55
import kotlin.coroutines.*
66

kotlinx-coroutines-debug/src/module-info.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@
33
requires java.instrument;
44
requires kotlin.stdlib;
55
requires kotlinx.coroutines.core;
6-
requires net.bytebuddy;
7-
requires net.bytebuddy.agent;
8-
requires org.junit.jupiter.api;
9-
requires org.junit.platform.commons;
6+
requires static net.bytebuddy;
7+
requires static net.bytebuddy.agent;
8+
requires static org.junit.jupiter.api;
9+
requires static org.junit.platform.commons;
1010

11-
// exports kotlinx.coroutines.debug; // already exported by kotlinx.coroutines.core
11+
exports kotlinx.coroutines.debug;
1212
exports kotlinx.coroutines.debug.junit4;
1313
exports kotlinx.coroutines.debug.junit5;
1414
}

0 commit comments

Comments
 (0)