Skip to content

Commit

Permalink
Always copy icudtl.dat with skiko.dll (#131)
Browse files Browse the repository at this point in the history
This change fixes an exception,
when Skiko is loaded multiple times
in a multi-classloader environment on Windows:

java.lang.RuntimeException: Can't wrap nullptr
	at org.jetbrains.skija.impl.Native.<init>(Native.java:13)
	at org.jetbrains.skija.impl.Managed.<init>(Managed.java:15)
	at org.jetbrains.skija.impl.Managed.<init>(Managed.java:11)
	at org.jetbrains.skija.paragraph.ParagraphBuilder.<init>(ParagraphBuilder.java:15)

The exception was happening, because
when `skiko.library.path` is not set,
icudtl.dat was copied into cacheDir,
even if the library was copied into copyDir.

Also icudtl.dat was always copied to `java.io.tmpdir`,
and a different version was not overwritten.
  • Loading branch information
AlexeyTsvetkov authored Jul 8, 2021
1 parent a124684 commit e0efcde
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 7 deletions.
14 changes: 12 additions & 2 deletions skiko/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -587,11 +587,21 @@ tasks.withType<AbstractPublishToMaven>().configureEach {
}
}

val skikoRuntimeDirForTests by project.tasks.registering(Copy::class) {
dependsOn(skikoJvmRuntimeJar)
from(zipTree(skikoJvmRuntimeJar.flatMap { it.archiveFile })) {
include("*.so")
include("*.dylib")
include("*.dll")
include("icudtl.dat")
}
destinationDir = project.buildDir.resolve("skiko-runtime-for-tests")
}
tasks.withType<Test>().configureEach {
dependsOn(project.tasks.withType(LinkSharedLibrary::class.java).single { it.name.contains(buildType.id) })
dependsOn(skikoRuntimeDirForTests)
dependsOn(skikoJvmRuntimeJar)
options {
val dir = skikoNativeLib.parentFile.absolutePath
val dir = skikoRuntimeDirForTests.map { it.destinationDir }.get()
systemProperty("skiko.library.path", dir)
val jar = skikoJvmRuntimeJar.get().outputs.files.files.single { it.name.endsWith(".jar")}
systemProperty("skiko.jar.path", jar.absolutePath)
Expand Down
13 changes: 9 additions & 4 deletions skiko/src/jvmMain/kotlin/org/jetbrains/skiko/Library.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import java.nio.file.StandardCopyOption
import java.util.concurrent.atomic.AtomicBoolean

object Library {
private val skikoLibraryPath = System.getProperty("skiko.library.path")
internal const val SKIKO_LIBRARY_PATH_PROPERTY = "skiko.library.path"
private val skikoLibraryPath = System.getProperty(SKIKO_LIBRARY_PATH_PROPERTY)
private val cacheRoot = "${System.getProperty("user.home")}/.skiko/"
private var copyDir: File? = null

Expand All @@ -20,8 +21,8 @@ object Library {
System.load(library.absolutePath)
} catch (e: UnsatisfiedLinkError) {
if (e.message?.contains("already loaded in another classloader") == true) {
val tempFile = File.createTempFile("skiko", if (hostOs.isWindows) ".dll" else "")
copyDir = tempFile.parentFile
copyDir = Files.createTempDirectory("skiko").toFile()
val tempFile = copyDir!!.resolve(library.name)
Files.copy(library.toPath(), tempFile.toPath(), StandardCopyOption.REPLACE_EXISTING)
tempFile.deleteOnExit()
System.load(tempFile.absolutePath)
Expand Down Expand Up @@ -78,7 +79,11 @@ object Library {
val library = unpackIfNeeded(cacheDir, platformName, false)
loadLibraryOrCopy(library)
if (icu != null) {
unpackIfNeeded(cacheDir, icu, false)
if (copyDir != null) {
unpackIfNeeded(copyDir!!, icu, true)
} else {
unpackIfNeeded(cacheDir, icu, false)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,49 @@ private fun testSkikoLoad(loader: ClassLoader) {
val tester = clazz.getDeclaredConstructor().newInstance()
val ptr = clazz.getMethod("run").invoke(tester) as Long
assert(ptr != 0L)

val paragraphPackage = "org.jetbrains.skija.paragraph"
val paragraphStyle = newInstance(loader, "$paragraphPackage.ParagraphStyle")
val fontCollection = newInstance(loader, "$paragraphPackage.FontCollection")
newInstance(loader, "$paragraphPackage.ParagraphBuilder", paragraphStyle, fontCollection)
}

private fun newInstance(loader: ClassLoader, fqName: String, vararg args: Any): Any {
val argsClasses = args.map { it.javaClass }.toTypedArray()
return loader.loadClass(fqName)
.getDeclaredConstructor(*argsClasses)
.newInstance(*args)
}

class SeveralClassloaders {
@Test
fun `load skiko in several classloaders`() {
fun `load skiko in several classloaders (with skiko path)`() {
check(skikoLibraryPath != null)
doTest()
}

@Test
fun `load skiko in several classloaders (without skiko path)`() {
val oldValue = skikoLibraryPath!!
skikoLibraryPath = null
try {
doTest()
} finally {
skikoLibraryPath = oldValue
}
}

private var skikoLibraryPath: String?
get() = System.getProperty(Library.SKIKO_LIBRARY_PATH_PROPERTY)
set(value) {
if (value != null) {
System.setProperty(Library.SKIKO_LIBRARY_PATH_PROPERTY, value)
} else {
System.clearProperty(Library.SKIKO_LIBRARY_PATH_PROPERTY)
}
}

private fun doTest() {
val threaded = false
val jar = System.getProperty("skiko.jar.path")
val stdlibClass = Class.forName("kotlin.jvm.internal.Intrinsics")
Expand Down

0 comments on commit e0efcde

Please sign in to comment.