Purge '.skiko' data dir if entries were unused for 31 days (default) (#1129)

## Release notes
### Features - Desktop
- Cleanup old unpacked binaries from `~/.skiko`. Use
`skiko.data.cleanup.days` to configure the retention period in days

dependencies.toml

@@ -11,6 +11,8 @@ githubApi = "1.329"
 gradleDownloadTask = "5.5.0"
 cryptoChecksumPlugin = "1.4.0"
 
+kotlinxBenchmark = "0.4.14"
+
 [libraries]
 coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "coroutines" }
 coroutines-core-jvm = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm", version.ref = "coroutines" }
@@ -26,3 +28,5 @@ gradleDownloadTask-gradlePlugin = { module = "de.undercouch:gradle-download-task
 githubApi = { module = "org.kohsuke:github-api", version.ref = "githubApi" }
 buildHelpers-publishing-gradlePlugin = { module = "org.jetbrains.compose.internal.build-helpers:publishing", version.ref = "buildHelpers-publishing" }
 crypto-checksum-gradlePlugin = { module = "gradle.plugin.org.gradle.crypto:checksum", version.ref = "cryptoChecksumPlugin" }
+kotlinx-benchmark-gradlePlugin = { module = "org.jetbrains.kotlinx:kotlinx-benchmark-plugin", version.ref = "kotlinxBenchmark" }
+kotlinx-benchmark-runtime = { module = "org.jetbrains.kotlinx:kotlinx-benchmark-runtime", version.ref = "kotlinxBenchmark"}

skiko/build.gradle.kts

@@ -19,6 +19,7 @@ plugins {
     `maven-publish`
     signing
     org.gradle.crypto.checksum
+    org.jetbrains.kotlinx.benchmark
 }
 
 if (supportAndroid) {
@@ -186,6 +187,10 @@ kotlin {
         implementation(libs.jetbrainsRuntime.api)
     }
 
+    skikoProjectContext.awtTestSourceSet?.dependencies {
+        implementation(libs.kotlinx.benchmark.runtime)
+    }
+
     skikoProjectContext.androidMainSourceSet?.dependencies {
         implementation(libs.coroutines.android)
     }
@@ -216,6 +221,21 @@ kotlin {
     }
 }
 
+/**
+ * Setup JVM benchmarks
+ */
+if (supportAwt) {
+    benchmark {
+        targets.register("awtTest")
+    }
+
+    /* Ensure that the benchmark task has the same classpath as the regular test task */
+    tasks.withType<JavaExec>().named { it == "awtTestBenchmark" }.configureEach {
+        classpath = project.files({ tasks.withType<Test>().named("awtTest").get().classpath })
+    }
+}
+
+
 if (supportAndroid) {
     // Android configuration, when available
     configure<LibraryExtension> {

skiko/buildSrc/build.gradle.kts

@@ -13,4 +13,5 @@ dependencies {
     implementation(libs.gradleDownloadTask.gradlePlugin)
     implementation(libs.githubApi)
     implementation(libs.crypto.checksum.gradlePlugin)
+    implementation(libs.kotlinx.benchmark.gradlePlugin)
 }

skiko/buildSrc/settings.gradle.kts

@@ -30,6 +30,7 @@ dependencyResolutionManagement {
             content {
                 includeGroupByRegex(".*com\\.gradle.*")
                 includeGroupByRegex(".*org\\.gradle.*")
+                includeModule("org.jetbrains.kotlinx", "kotlinx-benchmark-plugin")
             }
         }
 

skiko/buildSrc/src/main/kotlin/sourceHierarchy.kt

@@ -9,6 +9,8 @@ val SkikoProjectContext.jvmTestSourceSet get() = if (project.supportAwt) kotlin.
 
 val SkikoProjectContext.awtMainSourceSet get() = if (project.supportAwt) kotlin.sourceSets.getByName("awtMain") else null
 
+val SkikoProjectContext.awtTestSourceSet get() = if (project.supportAwt) kotlin.sourceSets.getByName("awtTest") else null
+
 val SkikoProjectContext.androidMainSourceSet get() = if (project.supportAndroid) kotlin.sourceSets.getByName("androidMain") else null
 
 val SkikoProjectContext.webTestSourceSet get() = if (project.supportWeb) kotlin.sourceSets.getByName("webTest") else null

skiko/src/awtTest/kotlin/org/jetbrains/skiko/LibraryLoadCleanupTest.kt

+package org.jetbrains.skiko
+
+import junit.framework.TestCase.assertFalse
+import junit.framework.TestCase.assertTrue
+import kotlinx.coroutines.*
+import kotlinx.coroutines.future.await
+import kotlinx.coroutines.test.runTest
+import java.nio.file.attribute.FileTime
+import java.time.Duration
+import java.time.Instant
+import kotlin.io.path.*
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.time.Duration.Companion.seconds
+
+class LibraryLoadCleanupTest {
+
+    /**
+     * Will check if launching skiko, using a .skiko data dir to extract the native binary to,
+     * will properly cleanup 'old' directories within the data dir.
+     * A temp directory will be created, we create two directories: old and new:
+     * The old one will be marked as '11' days old. The new one will be 9 days old.
+     * We start an isolate, pointing to this directory and setting the cleanup setting to 10 days.
+     * The test will assert that the 11 day old directory got deleted, whereas the 9 day old dir is still present.
+     */
+    @OptIn(ExperimentalPathApi::class)
+    @Test
+    fun `load library - purges old content in skiko data dir`() = runTest {
+        val tempDataDir = createTempDirectory()
+        currentCoroutineContext().job.invokeOnCompletion { tempDataDir.deleteRecursively() }
+
+        /*
+        Prepare a data dir to contain old and recent directories (note the '{Library.name}-' prefix)
+         */
+        val oldDirectory = tempDataDir.resolve("${Library.name}-old").createDirectories()
+        val newDirectory = tempDataDir.resolve("${Library.name}-skiko-new").createDirectories()
+
+        val oldDirectoryTime = FileTime.from(Instant.now() - Duration.ofDays(11))
+        val newDirectoryTime = FileTime.from(Instant.now() - Duration.ofDays(9))
+        oldDirectory.updateLastAccessTime(oldDirectoryTime)
+        newDirectory.updateLastAccessTime(newDirectoryTime)
+
+        assertTrue(oldDirectory.isDirectory())
+        assertTrue(newDirectory.isDirectory())
+
+        withContext(Dispatchers.IO) {
+            val process = ProcessBuilder(
+                ProcessHandle.current().info().command().get(),
+                "-cp", System.getProperty("java.class.path"),
+                "-Xmx64m",
+                "-Dskiko.data.path=${tempDataDir.absolutePathString()}",
+                "-Dskiko.data.cleanup.days=10", Isolate::class.java.name,
+            ).start()
+
+            launch {
+                process.inputStream.bufferedReader().forEachLine { line ->
+                    println(line)
+                }
+            }
+
+            launch {
+                process.errorStream.bufferedReader().forEachLine { line ->
+                    System.err.println(line)
+                }
+            }
+
+            withTimeout(15.seconds) {
+                process.onExit().await()
+                assertEquals(0, process.exitValue())
+            }
+        }
+
+        assertFalse("Expected 32 day old directory to be purged", oldDirectory.exists())
+        assertTrue("Expected 30 day 'new' directory to still exist", newDirectory.exists())
+        assertEquals(newDirectoryTime, newDirectory.getLastAccessTime())
+
+        /*
+        Additionally: Test our assertion that the .skiko data dir only contains
+        the .lock file and directories.
+         */
+        tempDataDir.listDirectoryEntries().forEach { file ->
+            when {
+                file.isReadable() && file.name == LockFile.skiko.name -> return@forEach
+                file.isReadable() && file.name == LockFile.angle.name -> return@forEach
+                file.isDirectory() -> return@forEach
+                else -> error("The cleanup implementation only expects directories and a .lock file\nfound: $file")
+            }
+        }
+    }
+
+    object Isolate {
+        @JvmStatic
+        fun main(args: Array<String>) {
+            Library.load()
+        }
+    }
+}

skiko/src/awtTest/kotlin/org/jetbrains/skiko/LibraryLoadStartupBenchmark.kt

+@file:OptIn(ExperimentalPathApi::class)
+
+package org.jetbrains.skiko
+
+import kotlinx.benchmark.*
+import kotlinx.benchmark.Setup
+import java.nio.file.Files
+import java.nio.file.Path
+import java.util.concurrent.TimeUnit
+import java.util.concurrent.atomic.AtomicInteger
+import kotlin.io.path.ExperimentalPathApi
+import kotlin.io.path.absolutePathString
+
+@Suppress("unused")
+@State(Scope.Benchmark)
+open class LibraryLoadStartupBenchmark {
+
+
+    lateinit var skikoDataDir: Array<Path>
+
+    val iteration = AtomicInteger(0)
+
+    @Setup
+    fun setup() {
+        skikoDataDir = Array(100) { Files.createTempDirectory("skiko-data-$it") }
+    }
+
+    @TearDown
+    fun tearDown() {
+        skikoDataDir.forEach { it.toFile().deleteRecursively() }
+    }
+
+    /**
+     * 19.11 | Mac M4
+     * N = 100
+     *   mean =      0.085 ±(99.9%) 0.013 s/op
+     *
+     * // Before .lock file
+     *   N = 100
+     *   mean =      0.085 ±(99.9%) 0.011 s/op
+     */
+    @Benchmark
+    @BenchmarkMode(Mode.SingleShotTime)
+    @Measurement(100)
+    fun warmStartup() {
+        startApplicationAndWait(skikoDataDir[0])
+    }
+
+    /**
+     * 19.11 | Mac M4
+     * N = 100
+     *   mean =      0.419 ±(99.9%) 0.005 s/op
+     *
+     * // Before .lock file
+     *   N = 100
+     *   mean =      0.423 ±(99.9%) 0.009 s/op
+     */
+    @Benchmark
+    @BenchmarkMode(Mode.SingleShotTime)
+    @Measurement(100)
+    fun coldStartup() {
+        startApplicationAndWait(skikoDataDir[iteration.getAndIncrement()])
+    }
+
+    private fun startApplicationAndWait(skikoDataDir: Path) {
+        val process = ProcessBuilder(
+            ProcessHandle.current().info().command().get(),
+            "-cp", System.getProperty("java.class.path"),
+            "-Xmx64m", "-Xms64m",
+            "-Dskiko.data.path=${skikoDataDir.absolutePathString()}",
+            Startup::class.java.name
+        ).start()
+
+        if (!process.waitFor(5, TimeUnit.SECONDS)) {
+            process.destroyForcibly()
+            throw AssertionError("Process didn't end in 5 seconds")
+        }
+
+        if (process.exitValue() != 0) {
+            val failure = process.errorStream.reader().readText()
+            val stdout = process.inputStream.reader().readText()
+            throw AssertionError("Process failed (${process.exitValue()})" + "\n" + failure + "\n" + stdout)
+        }
+    }
+
+    @Suppress("unused") // Launched in separate process
+    object Startup {
+        @JvmStatic
+        fun main(args: Array<String>) {
+            Library.load()
+            currentSystemTheme
+        }
+    }
+}

skiko/src/jvmMain/kotlin/org/jetbrains/skiko/AngleSupport.jvm.kt

@@ -10,6 +10,7 @@ private var loader = LibraryLoader(
     libEGLName,
     // libGLESv2 is not loaded explicitly in Skiko, it is loaded by libEGL
     additionalFile = System.mapLibraryName("libGLESv2"),
+    lockFile = LockFile.angle,
     init = {
         Library.staticLoad()
         if (!initAngleLibraryWindows(libEGLName)) {

skiko/src/jvmMain/kotlin/org/jetbrains/skiko/Library.kt

 package org.jetbrains.skiko
 
 import org.jetbrains.skia.Bitmap
-import java.util.concurrent.atomic.AtomicBoolean
 
 object Library {
+    internal val name: String = "skiko-$hostId"
+
     private var loader = LibraryLoader(
-        name = "skiko-$hostId",
+        name = name,
         additionalFile = if (hostOs.isWindows) "icudtl.dat" else null,
+        lockFile = LockFile.skiko,
         init = {
             Setup.init()
 

skiko/src/jvmMain/kotlin/org/jetbrains/skiko/LibraryLoader.kt

 package org.jetbrains.skiko
 
 import java.io.File
-import java.nio.channels.FileChannel
 import java.nio.file.Files
-import java.nio.file.Path
 import java.nio.file.StandardCopyOption
-import java.nio.file.StandardOpenOption.*
 import java.util.concurrent.atomic.AtomicReference
 import java.util.concurrent.locks.ReentrantLock
 import kotlin.concurrent.withLock
-import kotlin.io.path.createParentDirectories
 
 internal class LibraryLoader(
     /**
@@ -24,6 +20,11 @@ internal class LibraryLoader(
      */
     private val additionalFile: String? = null,
 
+    /**
+     * The lock file which shall be acquired if modifications on disk shall be synchronized across processes.
+     */
+    private val lockFile: LockFile,
+
     /**
      * Additional code that is called after successfully loading
      */
@@ -53,7 +54,7 @@ internal class LibraryLoader(
     private fun unpackIfNeeded(dest: File, resourceName: String, deleteOnExit: Boolean): File {
         val file = File(dest, resourceName)
         if (!file.exists()) {
-            withFileLock(dest.resolve(".lock").toPath()) {
+            lockFile.withLock {
                 if (file.exists()) return file
                 val tempFile = File.createTempFile("skiko", "", dest)
                 if (deleteOnExit)
@@ -72,7 +73,7 @@ internal class LibraryLoader(
      * or to wait for the loading to finish.
      * The reference will resolve to `null` if loading is done and callers do not need to wait anymore.
      */
-    private val loadingLock = AtomicReference(ReentrantLock())
+    private val loadingLock: AtomicReference<ReentrantLock?> = AtomicReference(ReentrantLock())
 
     /**
      * Load a native library finding it in multiple sources:
@@ -150,33 +151,24 @@ internal class LibraryLoader(
         )
         val hash = hashResourceStream.use { it.bufferedReader().readLine() }
 
-        val dataDir = File(File(SkikoProperties.dataPath), hash)
-        dataDir.mkdirs()
-        val library = unpackIfNeeded(dataDir, platformName, false)
-        val copyDir = loadLibraryOrCopy(library)
-        if (additionalFile != null) {
-            if (copyDir != null) {
-                // We made a duplicate to resolve classloader conflicts.
-                unpackIfNeeded(copyDir, additionalFile, true)
-            } else {
-                // Normal path where Skiko is loaded only once.
-                unpackIfNeeded(dataDir, additionalFile, false)
+        lockFile.withLock {
+            val dataDir = File(File(SkikoProperties.dataPath), "$name-$hash")
+            dataDir.mkdirs()
+            dataDir.toPath().updateLastAccessTime()
+            val library = unpackIfNeeded(dataDir, platformName, false)
+
+            val copyDir = loadLibraryOrCopy(library)
+            if (additionalFile != null) {
+                if (copyDir != null) {
+                    // We made a duplicate to resolve classloader conflicts.
+                    unpackIfNeeded(copyDir, additionalFile, true)
+                } else {
+                    // Normal path where Skiko is loaded only once.
+                    unpackIfNeeded(dataDir, additionalFile, false)
+                }
             }
         }
-    }
-}
 
-/**
- * Simple lockfile utility which ensures that the lockfile at the given [path] exists and is locked properly.
- * Note: This method cannot be re-entered recusrively
- * Note: The same process can only take a given lock once
- */
-internal inline fun <T> withFileLock(path: Path, action: () -> T): T {
-    path.createParentDirectories()
-    return FileChannel.open(path, READ, WRITE, CREATE).use { channel ->
-        val lock = channel.lock()
-        lock.use {
-            action()
-        }
+        enqueueSkikoDataDirCleanupIfNecessary(lockFile, "$name-*")
     }
 }

skiko/src/jvmMain/kotlin/org/jetbrains/skiko/LockFile.kt

+package org.jetbrains.skiko
+
+import org.jetbrains.skiko.SkikoProperties.dataPath
+import java.nio.channels.FileChannel
+import java.nio.channels.OverlappingFileLockException
+import java.nio.file.Path
+import java.nio.file.StandardOpenOption.*
+import kotlin.io.path.Path
+import kotlin.io.path.createParentDirectories
+import kotlin.io.path.name
+import kotlin.use
+
+
+/**
+ * We do use pre-defined lock files to synchronize file system modifications across processes.
+ * Note: The lock files shall be used for different purposes
+ * Note2: We're relying on String literals and String interning to share a monitor for the entire VM,
+ * this shall allow proper locking, even when skiko is loaded in entire isolation, in different ClassLoaders
+ */
+internal class LockFile private constructor(private val lockfile: Path, private val monitor: Any) {
+    val name: String = lockfile.name
+
+    inline fun <T> withLock(action: () -> T): T = withLockFile(monitor, lockfile, action)
+
+    companion object {
+        /**
+         * Lock file used to synchronize modifications to the 'skiko' native library
+         */
+        val skiko = LockFile(Path(dataPath, ".skiko.lock"), ".skiko.lock.monitor".intern())
+
+        /**
+         * Lock file used to synchronize modifications to the 'angle' native library
+         */
+        val angle = LockFile(Path(dataPath, ".angle.lock"), ".angle.lock.monitor".intern())
+    }
+}
+
+
+/**
+ * Runs the given [action] by capturing a file lock on the given [lockfile].
+ * Note: The provided [monitor] should is used to synchronize the lock within the current process.
+ * It is advisable to use a globally available object.
+ */
+private inline fun <T> withLockFile(monitor: Any, lockfile: Path, action: () -> T): T {
+    /**
+     * If we acquired the mutex, then we can be sure that we also have acquired the lockfile, which won't
+     * allow to re-enter, therefore, we can just return early
+     */
+    if (Thread.holdsLock(monitor)) {
+        return action()
+    }
+
+    synchronized(monitor) {
+        lockfile.createParentDirectories()
+        var attempts = 0
+
+        while (true) {
+            try {
+                return FileChannel.open(lockfile, READ, WRITE, CREATE).use { channel ->
+                    val lock = channel.lock()
+                    lock.use {
+                        action()
+                    }
+                }
+            } catch (_: OverlappingFileLockException) {
+                /**
+                 * Overlapping file lock exception can happen if our current process is trying to capture
+                 * the file lock while another thread already captured this lock,
+                 * note: this will only fail if the `dataPathMutex` of another thread was either not
+                 * interned correctly, or if another thread captured the lock without using the mutex
+                 * (maybe outside of this function?)
+                 */
+                if (attempts % 128 == 0) {
+                    Logger.debug { "Trying to acquire lock '$lockfile'; Waiting for another thread to release the lock..." }
+                }
+
+                Thread.sleep(64)
+                attempts++
+            }
+        }
+    }
+}

skiko/src/jvmMain/kotlin/org/jetbrains/skiko/SkikoProperties.kt

@@ -32,6 +32,11 @@ object SkikoProperties {
      */
     val dataPath: String get() = getProperty("skiko.data.path") ?: "${getProperty("user.home")}/.skiko/"
 
+    /**
+     * Purge data inside the [dataPath] if it is not used/older than this 'days'
+     */
+    val dataCleanupDays: Int get() = getProperty("skiko.data.cleanup.days")?.toInt() ?: 31
+
     val vsyncEnabled: Boolean get() = getProperty("skiko.vsync.enabled")?.toBoolean() ?: true
 
     val frameBuffering: FrameBuffering get() {

skiko/src/jvmMain/kotlin/org/jetbrains/skiko/cleanup.kt

+@file:OptIn(ExperimentalCoroutinesApi::class)
+
+package org.jetbrains.skiko
+
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import java.io.FileNotFoundException
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.attribute.BasicFileAttributeView
+import java.nio.file.attribute.BasicFileAttributes
+import java.nio.file.attribute.FileTime
+import java.time.Duration
+import java.time.Instant
+import java.util.concurrent.LinkedBlockingQueue
+import java.util.concurrent.ThreadPoolExecutor
+import java.util.concurrent.TimeUnit
+import kotlin.concurrent.thread
+import kotlin.io.path.*
+
+
+/**
+ * Cleanup actions will be enqueued on this executor if needed.
+ * Note: We're not using [java.util.concurrent.Executors.newSingleThreadExecutor] here,
+ * as this executor will keep the provided thread (and therefore JVM process) until it is shut down
+ * Using a daemon thread is however not desirable for the cleanup as well, as it will just be stopped
+ * during shutdown, leaving us with corrupted states.
+ *
+ * Therefore, a threadpool executor which only spawns a single thraed when needed and closes this thread
+ * once all scheduled work is done seems like the correct choice.
+ */
+private val cleanupExecutor = ThreadPoolExecutor(
+    /* corePoolSize = */ 0, /* maximumPoolSize = */1,
+    /* keepAliveTime = */0, /* unit = */TimeUnit.SECONDS,
+    /* workQueue = */LinkedBlockingQueue()
+) { runnable ->
+    thread(start = false, isDaemon = false, name = "skiko-cleanup-thread") { runnable.run() }
+}
+
+/**
+ * Will asynchronously clean up 'stale' entries in the [SkikoProperties.dataPath]:
+ * If directories are older than [SkikoProperties.dataCleanupDays], then they will be deleted.
+ *
+ * Cleanup Rule:
+ * Everything directory in the [SkikoProperties.dataPath] is expected to maintain a 'last access time'
+ * (see [BasicFileAttributeView.readAttributes]). If a given directory was not accessed (is older than) the
+ * [SkikoProperties.dataCleanupDays], then it will be deleted.
+ *
+ * The cleaning can be entirely disabled by setting [SkikoProperties.dataCleanupDays] to `-1`.
+ */
+internal fun enqueueSkikoDataDirCleanupIfNecessary(lockFile: LockFile, glob: String) {
+    if (SkikoProperties.dataCleanupDays < 0) return
+    cleanupExecutor.execute {
+        try {
+            doCleanupSkikoDataDir(lockFile, glob)
+        } catch (t: Throwable) {
+            Logger.error(t) { "Exception occurred during .skiko cleanup" }
+        }
+    }
+}
+
+private fun doCleanupSkikoDataDir(lockFile: LockFile, glob: String) {
+    val dir = Path(SkikoProperties.dataPath)
+    dir.listDirectoryEntries(glob).forEach { entry ->
+        if (!entry.isDirectory()) return@forEach
+
+        lockFile.withLock {
+            /*
+            Catch the case where another process could _eventually_ delete the directory right before us
+            trying to read the 'lastAccessTime'
+             */
+            val duration = try {
+                entry.timeSinceLastAccessed()
+            } catch (_: FileNotFoundException) {
+                return@forEach
+            }
+
+            if (duration.toDays() > SkikoProperties.dataCleanupDays) {
+                if (!entry.exists()) return@withLock
+                Logger.debug { "Cleaning up '${entry.name}' in '.${dir.name}' directory after ${duration.toDays()}" }
+                entry.toFile().deleteRecursively()
+            }
+        }
+    }
+}
+
+private fun Path.timeSinceLastAccessed(): Duration {
+    val lastModifiedTime = Files.readAttributes(this, BasicFileAttributes::class.java).lastAccessTime().toInstant()
+    return Duration.between(lastModifiedTime, Instant.now())
+}
+
+internal fun Path.updateLastAccessTime(instant: Instant = Instant.now()) {
+    updateLastAccessTime(FileTime.from(instant))
+}
+
+internal fun Path.updateLastAccessTime(fileTime: FileTime) {
+    Files.getFileAttributeView(this, BasicFileAttributeView::class.java)
+        .setTimes(null, /* lastAccessTime */ fileTime, null)
+}
+
+internal fun Path.getLastAccessTime(): FileTime =
+    Files.readAttributes(this, BasicFileAttributes::class.java).lastAccessTime()