diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 6141a58535c..c810bb16651 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -3560,6 +3560,10 @@ if (PlatformInfo.isAppleTarget(project)) { it.headers "$projectDir/framework/gh3343/objclib.h" it.linkerOpts "-lobjcgh3343" } + createInterop("objc_illegal_sharing_with_weak") { + it.defFile 'interop/objc/illegal_sharing_with_weak/objclib.def' + it.headers "$projectDir/interop/objc/illegal_sharing_with_weak/objclib.h" + } } createInterop("withSpaces") { @@ -3913,6 +3917,18 @@ if (PlatformInfo.isAppleTarget(project)) { interop = 'objcKt34467' goldValue = "OK\n42\n" } + + interopTest("interop_objc_illegal_sharing_with_weak") { + source = "interop/objc/illegal_sharing_with_weak/main.kt" + interop = 'objc_illegal_sharing_with_weak' + + expectedExitStatusChecker = { it != 0 } + outputChecker = { + it.startsWith("Before") && // Should crash after "Before" + !it.contains("After") && // But before "After" + it.contains("isObjectAliveShouldCrash") // And should contain stack trace. + } + } } standaloneTest("jsinterop_math") { @@ -4479,4 +4495,4 @@ if (target.family == Family.IOS && (target.architecture == ARM32 || target.archi it instanceof KonanLinkTest } .forEach { it.enabled = false } -} \ No newline at end of file +} diff --git a/backend.native/tests/interop/objc/illegal_sharing_with_weak/main.kt b/backend.native/tests/interop/objc/illegal_sharing_with_weak/main.kt new file mode 100644 index 00000000000..d615e3bbc88 --- /dev/null +++ b/backend.native/tests/interop/objc/illegal_sharing_with_weak/main.kt @@ -0,0 +1,28 @@ +import kotlin.native.concurrent.* +import kotlinx.cinterop.autoreleasepool +import objclib.* + +fun main() { + autoreleasepool { + run() + } +} + +private class NSObjectImpl : NSObject() { + var x = 111 +} + +fun run() = withWorker { + val obj = NSObjectImpl() + setObject(obj) + + println("Before") + val isAlive = try { + execute(TransferMode.SAFE, {}) { + isObjectAliveShouldCrash() + }.result + } catch (e: Throwable) { + false + } + println("After $isAlive") +} diff --git a/backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.def b/backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.def new file mode 100644 index 00000000000..758f44a49a2 --- /dev/null +++ b/backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.def @@ -0,0 +1,2 @@ +language = Objective-C +headerFilter = **/objclib.h diff --git a/backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.h b/backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.h new file mode 100644 index 00000000000..b44bffe7dc3 --- /dev/null +++ b/backend.native/tests/interop/objc/illegal_sharing_with_weak/objclib.h @@ -0,0 +1,13 @@ +#import + +static NSObject* __weak globalObject = nil; + +void setObject(NSObject* obj) { + globalObject = obj; +} + +// Make sure this function persists, because the test expects to find this function in the stack trace. +__attribute__((noinline)) +bool isObjectAliveShouldCrash() { + return globalObject != nil; +} diff --git a/runtime/src/main/cpp/Exceptions.cpp b/runtime/src/main/cpp/Exceptions.cpp index ad535102cc5..6ae34e90756 100644 --- a/runtime/src/main/cpp/Exceptions.cpp +++ b/runtime/src/main/cpp/Exceptions.cpp @@ -103,6 +103,14 @@ _Unwind_Reason_Code unwindCallback( } #endif +THREAD_LOCAL_VARIABLE bool disallowSourceInfo = false; + +SourceInfo getSourceInfo(KConstRef stackTrace, int index) { + return disallowSourceInfo + ? SourceInfo { .fileName = nullptr, .lineNumber = -1, .column = -1 } + : Kotlin_getSourceInfo(*PrimitiveArrayAddressOfElementAt(stackTrace->array(), index)); +} + } // namespace extern "C" { @@ -172,7 +180,7 @@ OBJ_GETTER(GetStackTraceStrings, KConstRef stackTrace) { RuntimeCheck(symbols != nullptr, "Not enough memory to retrieve the stacktrace"); for (int index = 0; index < size; ++index) { - auto sourceInfo = Kotlin_getSourceInfo(*PrimitiveArrayAddressOfElementAt(stackTrace->array(), index)); + auto sourceInfo = getSourceInfo(stackTrace, index); const char* symbol = symbols[index]; const char* result; char line[1024]; @@ -302,4 +310,8 @@ void SetKonanTerminateHandler() { #endif // KONAN_OBJC_INTEROP -} // extern "C" \ No newline at end of file +} // extern "C" + +void DisallowSourceInfo() { + disallowSourceInfo = true; +} diff --git a/runtime/src/main/cpp/Exceptions.h b/runtime/src/main/cpp/Exceptions.h index 4455f5d61d9..e71cabe6622 100644 --- a/runtime/src/main/cpp/Exceptions.h +++ b/runtime/src/main/cpp/Exceptions.h @@ -67,4 +67,7 @@ void PrintThrowable(KRef); } // extern "C" #endif +// It's not always safe to extract SourceInfo during unhandled exception termination. +void DisallowSourceInfo(); + #endif // RUNTIME_NAMES_H diff --git a/runtime/src/main/cpp/ObjCInterop.mm b/runtime/src/main/cpp/ObjCInterop.mm index 87f4927e03a..f74a85433a3 100644 --- a/runtime/src/main/cpp/ObjCInterop.mm +++ b/runtime/src/main/cpp/ObjCInterop.mm @@ -102,17 +102,14 @@ BOOL _tryRetainImp(id self, SEL _cmd) { return getBackRef(self)->tryAddRef(); } catch (ExceptionObjHolder& e) { // TODO: check for IncorrectDereferenceException and possible weak property access - @try { - // try and catch with objc_terminate: this is a workaround to terminate immediately - // with libc default_terminate_handler() to be called instead of custom `TerminateWithUnhandledException`. - // See `KonanTerminateHandler`. - // TerminateWithUnhandledException shall not be used here because in debug mode it uses - // CoreSymbolication framework (CSSymbolOwnerGetSymbolWithAddress) which fails at recursive retain lock. - [NSException raise:NSGenericException - format:@"Possible illegal attempt to access weak property from non-owning thread"]; - } @catch (...) { - objc_terminate(); - } + // Cannot use SourceInfo here, because CoreSymbolication framework (CSSymbolOwnerGetSymbolWithAddress) + // fails at recursive retain lock. Similarly, cannot use objc exception here, because its unhandled + // exception handler might fail at recursive retain lock too. + // TODO: Refactor to be more explicit. Instead of relying on an unhandled exception termination + // (and effectively setting a global to alter its behavior), just call an appropriate termination + // function by hand. + DisallowSourceInfo(); + std::terminate(); } }