Skip to content

Commit

Permalink
Make Skia context usage a critical section on iOS. (#896)
Browse files Browse the repository at this point in the history
## Proposed Changes

Since there are no reliable repros, it's a speculative fix for [the
crash](JetBrains/compose-multiplatform#3862)
on iOS.
It's based on an assumption that the case for the crash is caused by
render command encoding in a separate thread being performed after (or
in parallel) with the context disposal on the main thread which leads to
incorrect state inside Skia.

## Testing

Test: see if issues persists.

## Issues Fixed

Fixes: JetBrains/compose-multiplatform#3862

## Note
Skia is supposed to handle internal resources based on reference
counting and assumed scenario shouldn't lead to the crash, since the
context should be indirectly retained by the moment encoding starts.
Revert if issue persists because that logic would be redundant.
  • Loading branch information
elijah-semyonov committed Nov 6, 2023
1 parent 93a43bf commit a985c43
Showing 1 changed file with 16 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ internal class MetalRedrawer(
private var lastRenderTimestamp: NSTimeInterval = CACurrentMediaTime()
private val pictureRecorder = PictureRecorder()

/**
* Lock used to avoid skia context being disposed in the middle of rendering encoding on a separate thread.
*/
private val disposeLock = NSLock()

// Semaphore for preventing command buffers count more than swapchain size to be scheduled/executed at the same time
private val inflightSemaphore =
dispatch_semaphore_create(metalLayer.maximumDrawableCount.toLong())
Expand Down Expand Up @@ -284,7 +289,7 @@ internal class MetalRedrawer(
caDisplayLink.addToRunLoop(NSRunLoop.mainRunLoop, NSRunLoop.mainRunLoop.currentMode)
}

fun dispose() {
fun dispose() = disposeLock.doLocked {
check(caDisplayLink != null) { "MetalRedrawer.dispose() was called more than once" }

applicationStateListener.dispose()
Expand Down Expand Up @@ -432,7 +437,16 @@ internal class MetalRedrawer(
} else {
dispatch_async(renderingDispatchQueue) {
autoreleasepool {
encodeAndPresentBlock()
disposeLock.doLocked {
if (caDisplayLink == null) {
// Was disposed before render encoding started
picture.close()
surface.close()
renderTarget.close()
} else {
encodeAndPresentBlock()
}
}
}
}
}
Expand Down

0 comments on commit a985c43

Please sign in to comment.