Skip to content

Commit 3eccb11

Browse files
SvyatoslavScherbinaprojedi
authored andcommitted
[K/N] Enforce the native thread state for skiko's _nFlushAndSubmit
`org_jetbrains_skia_DirectContext__1nFlushAndSubmit` is a function from Skiko written in C++ and called from Kotlin through a `@SymbolName external fun`. As discovered in KT-75895, this function might block and wait for another Kotlin thread. Therefore, it violates the contract of `SymbolName`. Switching the thread state to "native" is required when calling such a function, which `SymbolName` doesn't do. This commit adds a hack to the compiler: when calling a function with such a symbol name, enforce thread state switching. The hack is configurable: it is added in the form of a binary option, `-Xbinary=forceNativeThreadStateForFunctions=fun1;fun2`, with `org_jetbrains_skia_DirectContext__1nFlushAndSubmit` as the default value. It can also be disabled with `-Xbinary=forceNativeThreadStateForFunctions=`. Specifying a value for `forceNativeThreadStateForFunctions` disabled compiler caches (since callsites to listed functions might be in the caches). ^KT-77489 Fixed
1 parent 5524d1e commit 3eccb11

File tree

3 files changed

+40
-5
lines changed

3 files changed

+40
-5
lines changed

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BinaryOptions.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ object BinaryOptions : BinaryOptionRegistry() {
8686

8787
val enableSafepointSignposts by booleanOption()
8888

89+
val forceNativeThreadStateForFunctions by listOption(StringValueParser)
90+
8991
val packFields by booleanOption()
9092

9193
val cInterfaceMode by option<CInterfaceGenerationMode>()

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,12 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration
277277
}
278278
} ?: false // Disabled by default because of KT-68928
279279

280+
val forceNativeThreadStateForFunctions: Set<String> =
281+
configuration.get(BinaryOptions.forceNativeThreadStateForFunctions)?.toSet()
282+
?: setOf(
283+
"org_jetbrains_skia_DirectContext__1nFlushAndSubmit", // KT-75895, KT-79384
284+
)
285+
280286
val globalDataLazyInit: Boolean by lazy {
281287
configuration.get(BinaryOptions.globalDataLazyInit) ?: true
282288
}
@@ -542,6 +548,7 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration
542548
internal val ignoreCacheReason = when {
543549
optimizationsEnabled -> "for optimized compilation"
544550
runtimeLogsEnabled -> "with runtime logs"
551+
configuration.get(BinaryOptions.forceNativeThreadStateForFunctions) != null -> "with non-default forceNativeThreadStateForFunctions"
545552
else -> null
546553
}
547554

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2597,15 +2597,41 @@ internal class CodeGeneratorVisitor(
25972597
resultLifetime: Lifetime, resultSlot: LLVMValueRef?): LLVMValueRef {
25982598
check(!function.isTypedIntrinsic)
25992599

2600-
val needsNativeThreadState = function.needsNativeThreadState
2601-
val exceptionHandler = function.annotations.findAnnotation(RuntimeNames.filterExceptions)?.let {
2602-
val foreignExceptionMode = ForeignExceptionMode.byValue(it.getAnnotationValueOrNull<String>("mode"))
2600+
val foreignExceptionModeFromAnnotation = function.annotations.findAnnotation(RuntimeNames.filterExceptions)?.let {
2601+
ForeignExceptionMode.byValue(it.getAnnotationValueOrNull<String>("mode"))
2602+
}
2603+
2604+
val needsNativeThreadState: Boolean
2605+
val filterExceptionWith: ForeignExceptionMode.Mode?
2606+
2607+
if (llvmCallable.name in context.config.forceNativeThreadStateForFunctions) {
2608+
// This is a quick hack for functions that break the contract of `SymbolName` by being blocking,
2609+
// and therefore need the native thread state.
2610+
// See e.g., KT-75895 and KT-79384.
2611+
needsNativeThreadState = true
2612+
2613+
// Switching to the native thread state requires a filteringExceptionHandler,
2614+
// so we enforce one here.
2615+
// Otherwise, nothing will switch the state back to runnable in case of exception.
2616+
// A more flexible approach can be implemented but is not necessary for this quick hack.
2617+
filterExceptionWith = foreignExceptionModeFromAnnotation ?: ForeignExceptionMode.Mode.TERMINATE
2618+
} else {
2619+
needsNativeThreadState = function.needsNativeThreadState
2620+
filterExceptionWith = foreignExceptionModeFromAnnotation
2621+
}
2622+
2623+
val exceptionHandler = if (filterExceptionWith != null) {
26032624
functionGenerationContext.filteringExceptionHandler(
26042625
currentCodeContext.exceptionHandler,
2605-
foreignExceptionMode,
2626+
filterExceptionWith,
26062627
needsNativeThreadState
26072628
)
2608-
} ?: currentCodeContext.exceptionHandler
2629+
} else {
2630+
check(!needsNativeThreadState) {
2631+
"${llvmCallable.name} needs native thread state, but doesn't have a filtering exception handler"
2632+
}
2633+
currentCodeContext.exceptionHandler
2634+
}
26092635

26102636
if (needsNativeThreadState) {
26112637
functionGenerationContext.switchThreadState(ThreadState.Native)

0 commit comments

Comments
 (0)