Unverified Commit dc985814 authored by Igor Demin's avatar Igor Demin Committed by GitHub

Linux fix deadlock when we close a window (#646)

Deadlock:
[AWTThread] awtLock -> SkiaLayer.dispose -> LinuxOpenGLRedrawer.dispose -> we are here -> lockLinuxDrawingSurface -> awtLock
[refreshRate updater thread] -> lockLinuxDrawingSurface -> we are here -> awtLock

We moved refreshRate to another thread, and check it only once a second, because it is not fast. I mistakenly thought in the past, that it is very slow on Linux (2ms), but it appears it was because of the awtLock itself. In the AWT thread it take 0.2ms, that is also slow for just one functions, but bearable, if it called only once every second.

Alternative is to subscribe to the native XRRScreenChangeNotifyEvent: https://www.x.org/releases/current/doc/man/man3/Xrandr.3.xhtml. But it seems X window system can have only one event queue associated with the window, and we already have one inside AWT, and we don't have access to it.
parent 2295bace
package org.jetbrains.skiko
import kotlin.time.ExperimentalTime
internal const val MinMainstreamMonitorRefreshRate = 60.0
@OptIn(ExperimentalTime::class)
internal fun HardwareLayer.getDisplayRefreshRate(): Double {
// We use different method for Linux, because it.displayMode.refreshRate returns always a wrong value: 50 (probably because of the using the old xrandr API)
return if (hostOs == OS.Linux) {
......
......@@ -8,7 +8,6 @@ import java.awt.event.InputMethodEvent
import java.beans.PropertyChangeEvent
import javax.accessibility.Accessible
import javax.accessibility.AccessibleContext
import kotlin.time.ExperimentalTime
internal open class HardwareLayer(
externalAccessibleFactory: ((Component) -> Accessible)? = null
......@@ -128,21 +127,7 @@ internal open class HardwareLayer(
}
}
/**
* HardwareLayer should not dispose native resources while [scope] is active.
*
* So wait for scope cancellation in dispose method:
* ```
* runBlocking {
* frameJob.cancelAndJoin()
* }
* ```
*
* Can be accessed from multiple threads.
*/
@OptIn(ExperimentalTime::class)
@Suppress("UNUSED_PARAMETER")
internal fun FrameLimiter(
internal fun layerFrameLimiter(
scope: CoroutineScope,
component: HardwareLayer,
onNewFrameLimit: (frameLimit: Double) -> Unit = {}
......@@ -155,11 +140,12 @@ internal fun FrameLimiter(
val frames = Channel<Unit>(Channel.CONFLATED)
frames.trySend(Unit)
scope.launch {
// Use UI thread, because getDisplayRefreshRate uses UI lock anyway, and there is no point to call it in
// a separate thread. Besides that, if we call it in a separate thread, we can have deadlocks
scope.launch(MainUIDispatcher) {
while (true) {
frames.receive()
// TODO will lockLinuxDrawingSurface inside getDisplayRefreshRate can cause draw lock too?
// it takes 2ms on my machine on Linux (0.01ms on macOs, 0.1ms on Windows)
// on my machine it takes 0.2ms on Linux, 0.01ms on macOs, 0.1ms on Windows
state.frameLimit = component.getDisplayRefreshRate()
onNewFrameLimit(state.frameLimit)
delay(1000)
......@@ -167,7 +153,7 @@ internal fun FrameLimiter(
}
return FrameLimiter(
scope,
scope + Dispatchers.IO,
frameMillis = {
frames.trySend(Unit)
(1000 / state.frameLimit).toLong()
......
......@@ -3,7 +3,7 @@ package org.jetbrains.skiko.redrawer
import org.jetbrains.skia.Surface
import kotlinx.coroutines.*
import org.jetbrains.skiko.*
import org.jetbrains.skiko.FrameLimiter
import org.jetbrains.skiko.layerFrameLimiter
import org.jetbrains.skiko.context.DirectSoftwareContextHandler
internal abstract class AbstractDirectSoftwareRedrawer(
......@@ -15,7 +15,7 @@ internal abstract class AbstractDirectSoftwareRedrawer(
override val renderInfo: String get() = contextHandler.rendererInfo()
private val frameJob = Job()
private val frameLimiter = FrameLimiter(CoroutineScope(Dispatchers.IO + frameJob), layer.backedLayer)
private val frameLimiter = layerFrameLimiter(CoroutineScope(frameJob), layer.backedLayer)
private val frameDispatcher = FrameDispatcher(MainUIDispatcher) {
if (properties.isVsyncEnabled && properties.isVsyncFramelimitFallbackEnabled) {
frameLimiter.awaitNextFrame()
......@@ -50,12 +50,10 @@ internal abstract class AbstractDirectSoftwareRedrawer(
}
open fun finishFrame(surface: Long) = finishFrame(device, surface)
override fun dispose() {
frameJob.cancel()
frameDispatcher.cancel()
contextHandler.dispose()
disposeDevice(device)
runBlocking {
frameJob.cancelAndJoin()
}
super.dispose()
}
......
......@@ -38,8 +38,8 @@ internal class LinuxOpenGLRedrawer(
private val frameJob = Job()
@Volatile
private var frameLimit = 0.0
private val frameLimiter = FrameLimiter(
CoroutineScope(Dispatchers.IO + frameJob),
private val frameLimiter = layerFrameLimiter(
CoroutineScope(frameJob),
layer.backedLayer,
onNewFrameLimit = { frameLimit = it }
)
......@@ -57,6 +57,7 @@ internal class LinuxOpenGLRedrawer(
override fun dispose() {
check(!isDisposed) { "LinuxOpenGLRedrawer is disposed" }
frameJob.cancel()
layer.backedLayer.lockLinuxDrawingSurface {
// makeCurrent is mandatory to destroy context, otherwise, OpenGL will destroy wrong context (from another window).
// see the official example: https://www.khronos.org/opengl/wiki/Tutorial:_OpenGL_3.0_Context_Creation_(GLX)
......@@ -64,9 +65,6 @@ internal class LinuxOpenGLRedrawer(
contextHandler.dispose()
it.destroyContext(context)
}
runBlocking {
frameJob.cancelAndJoin()
}
super.dispose()
}
......
......@@ -2,7 +2,7 @@ package org.jetbrains.skiko.redrawer
import kotlinx.coroutines.*
import org.jetbrains.skiko.*
import org.jetbrains.skiko.FrameLimiter
import org.jetbrains.skiko.layerFrameLimiter
import org.jetbrains.skiko.context.SoftwareContextHandler
internal class SoftwareRedrawer(
......@@ -18,7 +18,7 @@ internal class SoftwareRedrawer(
override val renderInfo: String get() = contextHandler.rendererInfo()
private val frameJob = Job()
private val frameLimiter = FrameLimiter(CoroutineScope(Dispatchers.IO + frameJob), layer.backedLayer)
private val frameLimiter = layerFrameLimiter(CoroutineScope(frameJob), layer.backedLayer)
private val frameDispatcher = FrameDispatcher(MainUIDispatcher) {
if (properties.isVsyncEnabled && properties.isVsyncFramelimitFallbackEnabled) {
......@@ -36,11 +36,9 @@ internal class SoftwareRedrawer(
}
override fun dispose() {
frameJob.cancel()
frameDispatcher.cancel()
contextHandler.dispose()
runBlocking {
frameJob.cancelAndJoin()
}
super.dispose()
}
......
......@@ -14,7 +14,6 @@ private const val NanosecondsPerMillisecond = 1_000_000L
* (Windows has ~15ms precision by default, Linux/macOs ~2ms).
* FrameLimiter will try to delay frames as close as possible to [frameMillis], but not greater
*/
@OptIn(ExperimentalTime::class)
class FrameLimiter(
private val coroutineScope: CoroutineScope,
private val frameMillis: () -> Long,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment