From bb9a3819acfb5a07b251dc666bc2895bd0ba293c Mon Sep 17 00:00:00 2001 From: Yiqun Zhang Date: Mon, 18 May 2026 11:48:43 +0800 Subject: [PATCH 1/2] :bug: serialize Skia codec/bitmap close with in-flight readPixels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decoding frames on ioDispatcher (#4422) made codec.readPixels a synchronous JNI call that ignores coroutine cancellation, while DisposableEffect.onDispose closed bitmaps and produceState.awaitDispose closed the codec — both could free native pixel buffers under an active decode and crash the process. Fold codec + bitmap ownership into a single LaunchedEffect whose finally runs only after the last withContext returns, so close happens after readPixels regardless of when composition leaves. --- .../side/preview/SkiaAnimatedImageView.kt | 199 ++++++++---------- 1 file changed, 88 insertions(+), 111 deletions(-) diff --git a/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt b/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt index 37ecaae3e..2519ab0d3 100644 --- a/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt +++ b/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt @@ -9,16 +9,16 @@ import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf -import androidx.compose.runtime.produceState +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.geometry.Size +import androidx.compose.ui.graphics.ImageBitmap import androidx.compose.ui.graphics.asComposeImageBitmap import com.composables.icons.materialsymbols.MaterialSymbols import com.composables.icons.materialsymbols.rounded.Broken_image @@ -28,6 +28,7 @@ import com.crosspaste.ui.theme.AppUISize.tiny import com.crosspaste.utils.extension import com.crosspaste.utils.ioDispatcher import io.github.oshai.kotlinlogging.KotlinLogging +import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.delay import kotlinx.coroutines.withContext import okio.Path @@ -52,14 +53,15 @@ fun Path.isAnimatedImage(): Boolean = extension.lowercase() in animatedImageExte internal fun frameDurationMs(rawDurationMs: Int?): Int = (rawDurationMs ?: MIN_FRAME_DURATION_MS).coerceAtLeast(MIN_FRAME_DURATION_MS) -private sealed interface CodecState { - data object Loading : CodecState +private sealed interface FrameState { + data object Loading : FrameState - data object Failed : CodecState + data object Failed : FrameState data class Ready( - val codec: Codec, - ) : CodecState + val imageBitmap: ImageBitmap, + val srcSize: Size, + ) : FrameState } @Composable @@ -70,126 +72,101 @@ fun SkiaAnimatedImageView( isPlaying: Boolean, modifier: Modifier = Modifier, ) { - val state by produceCodecState(path) - - when (val current = state) { - CodecState.Loading -> LoadingIndicator() - CodecState.Failed -> BrokenIcon(path.name) - is CodecState.Ready -> - AnimatedFrames( - codec = current.codec, - targetSizePx = targetSizePx, - smartImageDisplayStrategy = smartImageDisplayStrategy, - isPlaying = isPlaying, - modifier = modifier, - contentDescription = path.name, - ) - } -} + var frameState by remember(path) { mutableStateOf(FrameState.Loading) } + val playing by rememberUpdatedState(isPlaying) + + // Codec + double-buffered bitmaps are owned by this single LaunchedEffect + // so the finally block runs only after any in-flight readPixels returns. + // readPixels is a synchronous JNI call that ignores coroutine cancellation, + // so releasing native resources from a sibling DisposableEffect.onDispose + // (or produceState's awaitDispose) could free pixel buffers under an + // active decode and crash with SIGSEGV. + LaunchedEffect(path) { + val codec = + withContext(ioDispatcher) { + runCatching { + val bytes = path.toFile().readBytes() + Codec.makeFromData(Data.makeFromBytes(bytes)) + }.onFailure { + logger.warn(it) { "Failed to decode animated image: $path" } + }.getOrNull() + } -@Composable -private fun AnimatedFrames( - codec: Codec, - targetSizePx: Size, - smartImageDisplayStrategy: SmartImageDisplayStrategy, - isPlaying: Boolean, - modifier: Modifier, - contentDescription: String, -) { - // Two bitmaps so the next frame can be decoded into the back buffer on IO - // while the front buffer keeps drawing. The swap publishes on the UI - // thread once readPixels returns. - val bitmaps = - remember(codec) { - Array(2) { Bitmap().apply { allocPixels(codec.imageInfo) } } + if (codec == null) { + frameState = FrameState.Failed + return@LaunchedEffect } - DisposableEffect(bitmaps) { - onDispose { bitmaps.forEach { it.close() } } - } - - val frameInfos = remember(codec) { codec.framesInfo } - val frameCount = remember(codec) { codec.frameCount.coerceAtLeast(1) } - - var frameIndex by remember(codec) { mutableIntStateOf(0) } - var frontIndex by remember(codec) { mutableIntStateOf(0) } - var frameTick by remember(codec) { mutableIntStateOf(0) } + val bitmaps = Array(2) { Bitmap().apply { allocPixels(codec.imageInfo) } } + try { + val frameInfos = codec.framesInfo + val frameCount = codec.frameCount.coerceAtLeast(1) + val srcSize = Size(codec.imageInfo.width.toFloat(), codec.imageInfo.height.toFloat()) - LaunchedEffect(codec, isPlaying, frameCount) { - // Prime frame 0 once per codec so something is drawable before the - // loop runs and so single-frame inputs (frameCount == 1) still render. - // Skipping on re-launch keeps the current frame visible when - // isPlaying toggles mid-playback. - if (frameTick == 0) { + // Prime frame 0 before publishing Ready so the first paint is the + // real first frame, not an empty bitmap. withContext(ioDispatcher) { runCatching { codec.readPixels(bitmaps[0], 0) } .onFailure { logger.warn(it) { "Failed to decode initial frame" } } } - frameTick = 1 - } - if (!isPlaying || frameCount <= 1) return@LaunchedEffect - - while (true) { - val durationMs = frameDurationMs(frameInfos.getOrNull(frameIndex)?.duration) - delay(durationMs.milliseconds) - val nextIndex = (frameIndex + 1) % frameCount - val backIndex = 1 - frontIndex - // Decode off the UI thread; LZW decompression on large frames - // can otherwise eat the next render window. - val success = - withContext(ioDispatcher) { - runCatching { codec.readPixels(bitmaps[backIndex], nextIndex) } - .onFailure { logger.warn(it) { "Failed to decode frame $nextIndex" } } - .isSuccess + var frameIndex = 0 + var frontIndex = 0 + frameState = FrameState.Ready(bitmaps[frontIndex].asComposeImageBitmap(), srcSize) + + if (frameCount <= 1) awaitCancellation() + + while (true) { + if (!playing) { + delay(MIN_FRAME_DURATION_MS.toLong().milliseconds) + continue + } + val durationMs = frameDurationMs(frameInfos.getOrNull(frameIndex)?.duration) + delay(durationMs.milliseconds) + if (!playing) continue + val nextIndex = (frameIndex + 1) % frameCount + val backIndex = 1 - frontIndex + // Decode off the UI thread; LZW decompression on large frames + // can otherwise eat the next render window. + val success = + withContext(ioDispatcher) { + runCatching { codec.readPixels(bitmaps[backIndex], nextIndex) } + .onFailure { logger.warn(it) { "Failed to decode frame $nextIndex" } } + .isSuccess + } + if (success) { + frameIndex = nextIndex + frontIndex = backIndex + frameState = FrameState.Ready(bitmaps[frontIndex].asComposeImageBitmap(), srcSize) } - if (success) { - frameIndex = nextIndex - frontIndex = backIndex - frameTick++ } + } finally { + bitmaps.forEach { it.close() } + codec.close() } } - val imageBitmap = - remember(frontIndex, frameTick) { - bitmaps[frontIndex].asComposeImageBitmap() - } - - val displayResult = - remember(codec, targetSizePx) { - smartImageDisplayStrategy.compute( - srcSize = Size(codec.imageInfo.width.toFloat(), codec.imageInfo.height.toFloat()), - dstSize = targetSizePx, + when (val current = frameState) { + FrameState.Loading -> LoadingIndicator() + FrameState.Failed -> BrokenIcon(path.name) + is FrameState.Ready -> { + val displayResult = + remember(current.srcSize, targetSizePx) { + smartImageDisplayStrategy.compute( + srcSize = current.srcSize, + dstSize = targetSizePx, + ) + } + Image( + bitmap = current.imageBitmap, + contentDescription = path.name, + contentScale = displayResult.contentScale, + alignment = displayResult.alignment, + modifier = modifier, ) } - - Image( - bitmap = imageBitmap, - contentDescription = contentDescription, - contentScale = displayResult.contentScale, - alignment = displayResult.alignment, - modifier = modifier, - ) -} - -@Composable -private fun produceCodecState(path: Path) = - produceState(initialValue = CodecState.Loading, key1 = path) { - val loaded = - withContext(ioDispatcher) { - runCatching { - val bytes = path.toFile().readBytes() - CodecState.Ready(Codec.makeFromData(Data.makeFromBytes(bytes))) - }.onFailure { - logger.warn(it) { "Failed to decode animated image: $path" } - }.getOrElse { CodecState.Failed } - } - value = loaded - awaitDispose { - if (loaded is CodecState.Ready) loaded.codec.close() - } } +} @Composable private fun LoadingIndicator() { From 85a49bdab5e4ec2e73cc5372540f3cca4174bf75 Mon Sep 17 00:00:00 2001 From: Yiqun Zhang Date: Mon, 18 May 2026 13:28:47 +0800 Subject: [PATCH 2/2] :hammer: tighten SkiaAnimatedImageView cleanup and pause - Nest try/finally so codec.close() still runs if Bitmap.allocPixels throws (e.g. OOM) before the inner try is entered. - Replace the 20ms !playing polling delay with snapshotFlow { playing } .first { it } so a hidden preview suspends instead of waking ~50x/s. --- .../side/preview/SkiaAnimatedImageView.kt | 81 ++++++++++--------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt b/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt index 2519ab0d3..c0bb20b7a 100644 --- a/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt +++ b/app/src/desktopMain/kotlin/com/crosspaste/ui/paste/side/preview/SkiaAnimatedImageView.kt @@ -15,6 +15,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.geometry.Size @@ -30,6 +31,7 @@ import com.crosspaste.utils.ioDispatcher import io.github.oshai.kotlinlogging.KotlinLogging import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext import okio.Path import org.jetbrains.skia.Bitmap @@ -97,51 +99,56 @@ fun SkiaAnimatedImageView( return@LaunchedEffect } - val bitmaps = Array(2) { Bitmap().apply { allocPixels(codec.imageInfo) } } try { - val frameInfos = codec.framesInfo - val frameCount = codec.frameCount.coerceAtLeast(1) - val srcSize = Size(codec.imageInfo.width.toFloat(), codec.imageInfo.height.toFloat()) - - // Prime frame 0 before publishing Ready so the first paint is the - // real first frame, not an empty bitmap. - withContext(ioDispatcher) { - runCatching { codec.readPixels(bitmaps[0], 0) } - .onFailure { logger.warn(it) { "Failed to decode initial frame" } } - } + val bitmaps = Array(2) { Bitmap().apply { allocPixels(codec.imageInfo) } } + try { + val frameInfos = codec.framesInfo + val frameCount = codec.frameCount.coerceAtLeast(1) + val srcSize = Size(codec.imageInfo.width.toFloat(), codec.imageInfo.height.toFloat()) + + // Prime frame 0 before publishing Ready so the first paint is the + // real first frame, not an empty bitmap. + withContext(ioDispatcher) { + runCatching { codec.readPixels(bitmaps[0], 0) } + .onFailure { logger.warn(it) { "Failed to decode initial frame" } } + } - var frameIndex = 0 - var frontIndex = 0 - frameState = FrameState.Ready(bitmaps[frontIndex].asComposeImageBitmap(), srcSize) + var frameIndex = 0 + var frontIndex = 0 + frameState = FrameState.Ready(bitmaps[frontIndex].asComposeImageBitmap(), srcSize) - if (frameCount <= 1) awaitCancellation() + if (frameCount <= 1) awaitCancellation() - while (true) { - if (!playing) { - delay(MIN_FRAME_DURATION_MS.toLong().milliseconds) - continue - } - val durationMs = frameDurationMs(frameInfos.getOrNull(frameIndex)?.duration) - delay(durationMs.milliseconds) - if (!playing) continue - val nextIndex = (frameIndex + 1) % frameCount - val backIndex = 1 - frontIndex - // Decode off the UI thread; LZW decompression on large frames - // can otherwise eat the next render window. - val success = - withContext(ioDispatcher) { - runCatching { codec.readPixels(bitmaps[backIndex], nextIndex) } - .onFailure { logger.warn(it) { "Failed to decode frame $nextIndex" } } - .isSuccess + while (true) { + if (!playing) { + // Suspend until isPlaying flips back to true instead of + // polling, so a hidden preview costs no wake-ups. + snapshotFlow { playing }.first { it } + continue + } + val durationMs = frameDurationMs(frameInfos.getOrNull(frameIndex)?.duration) + delay(durationMs.milliseconds) + if (!playing) continue + val nextIndex = (frameIndex + 1) % frameCount + val backIndex = 1 - frontIndex + // Decode off the UI thread; LZW decompression on large frames + // can otherwise eat the next render window. + val success = + withContext(ioDispatcher) { + runCatching { codec.readPixels(bitmaps[backIndex], nextIndex) } + .onFailure { logger.warn(it) { "Failed to decode frame $nextIndex" } } + .isSuccess + } + if (success) { + frameIndex = nextIndex + frontIndex = backIndex + frameState = FrameState.Ready(bitmaps[frontIndex].asComposeImageBitmap(), srcSize) } - if (success) { - frameIndex = nextIndex - frontIndex = backIndex - frameState = FrameState.Ready(bitmaps[frontIndex].asComposeImageBitmap(), srcSize) } + } finally { + bitmaps.forEach { it.close() } } } finally { - bitmaps.forEach { it.close() } codec.close() } }