Pass status code for ubrk_ operations to the front and fail in case we got...

Pass status code for ubrk_ operations to the front and fail in case we got non-zero error code (#418)

skiko/src/commonMain/kotlin/org/jetbrains/skia/BreakIterator.kt

 package org.jetbrains.skia
 
 import org.jetbrains.skia.impl.InteropPointer
+import org.jetbrains.skia.impl.InteropScope
 import org.jetbrains.skia.impl.Library.Companion.staticLoad
 import org.jetbrains.skia.impl.Managed
-import org.jetbrains.skia.impl.Stats
-import org.jetbrains.skia.impl.reachabilityBarrier
+import org.jetbrains.skia.impl.Native.Companion.NullPointer
 import org.jetbrains.skia.impl.NativePointer
-import org.jetbrains.skia.impl.getPtr
+import org.jetbrains.skia.impl.Stats
 import org.jetbrains.skia.impl.interopScope
+import org.jetbrains.skia.impl.reachabilityBarrier
 
 /**
  *
@@ -260,7 +261,7 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
          */
         fun makeCharacterInstance(locale: String? = null): BreakIterator {
             Stats.onNativeCall()
-            return BreakIterator(interopScope { _nMake(0, toInterop(locale))  }) // UBRK_CHARACTER
+            return BreakIterator(withErrorGuard("Failed to create character iterator") { _nMake(0, toInterop(locale), it)  }) // UBRK_CHARACTER
         }
         /**
          * Returns a new BreakIterator instance for word breaks for the given locale.
@@ -270,9 +271,7 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
          */
         fun makeWordInstance(locale: String? = null): BreakIterator {
             Stats.onNativeCall()
-            return BreakIterator(interopScope { _nMake(1, toInterop(locale)).also {
-                if (it == NullPointer) throw IllegalArgumentException("Cannot create word iterator")
-            } }) // UBRK_WORD
+            return BreakIterator(withErrorGuard("Failed to create word iterator") { _nMake(1, toInterop(locale), it) }) // UBRK_WORD
         }
         /**
          * Returns a new BreakIterator instance for line breaks for the given locale.
@@ -282,9 +281,7 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
          */
         fun makeLineInstance(locale: String? = null): BreakIterator {
             Stats.onNativeCall()
-            return BreakIterator(interopScope { _nMake(2, toInterop(locale)).also {
-                if (it == NullPointer) throw IllegalArgumentException("Cannot create line iterator")
-            } }) // UBRK_LINE
+            return BreakIterator(withErrorGuard("Failed to create line iterator")  { _nMake(2, toInterop(locale), it) }) // UBRK_LINE
         }
         /**
          * Returns a new BreakIterator instance for sentence breaks for the given locale.
@@ -294,9 +291,9 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
          */
         fun makeSentenceInstance(locale: String? = null): BreakIterator {
             Stats.onNativeCall()
-            return BreakIterator(interopScope { _nMake(3, toInterop(locale)).also {
-                if (it == NullPointer) throw IllegalArgumentException("Cannot create sentence iterator")
-            } }) // UBRK_SENTENCE
+            return BreakIterator(withErrorGuard("Failed to create sentence iterator")  {
+                _nMake(3, toInterop(locale), it)
+            }) // UBRK_SENTENCE
         }
 
         init {
@@ -315,7 +312,7 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
      */
     fun clone(): BreakIterator {
         Stats.onNativeCall()
-        return BreakIterator(_nClone(_ptr))
+        return BreakIterator(withErrorGuard("Failed to clone") { _nClone(_ptr, it) })
     }
 
     /**
@@ -511,7 +508,14 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
         try {
             Stats.onNativeCall()
             _text?.close()
-            _text = interopScope { U16String(_nSetText(_ptr, toInterop(text?.let { ShortArray(text.length) { text[it].code.toShort() } }), text?.length ?: 0)) }
+            _text = U16String(withErrorGuard("Failed to setText") {
+                _nSetText(
+                    _ptr,
+                    toInterop(text?.let { ShortArray(text.length) { text[it].code.toShort() } }),
+                    text?.length ?: 0,
+                    it
+                )
+            })
         } finally {
             reachabilityBarrier(this)
             reachabilityBarrier(_text)
@@ -523,15 +527,30 @@ class BreakIterator internal constructor(ptr: NativePointer) : Managed(ptr, _Fin
     }
 }
 
+private fun withErrorGuard(message: String, block: InteropScope.(InteropPointer) -> NativePointer): NativePointer {
+    val errorCode = IntArray(1)
+    return interopScope {
+        val handle = toInterop(errorCode)
+        val res = block.invoke(this, handle)
+        handle.fromInterop(errorCode)
+        if (errorCode[0] > 0) {
+            throw RuntimeException("$message; operation failed with status ${errorCode}")
+        }
+        if (res == NullPointer) {
+            throw IllegalArgumentException(message)
+        }
+        res
+    }
+}
 
 @ExternalSymbolName("org_jetbrains_skia_BreakIterator__1nGetFinalizer")
 private external fun BreakIterator_nGetFinalizer(): NativePointer
 
 @ExternalSymbolName("org_jetbrains_skia_BreakIterator__1nMake")
-private external fun _nMake(type: Int, locale: InteropPointer): NativePointer
+private external fun _nMake(type: Int, locale: InteropPointer, errorCode: InteropPointer): NativePointer
 
 @ExternalSymbolName("org_jetbrains_skia_BreakIterator__1nClone")
-private external fun _nClone(ptr: NativePointer): NativePointer
+private external fun _nClone(ptr: NativePointer, errorCode: InteropPointer): NativePointer
 
 @ExternalSymbolName("org_jetbrains_skia_BreakIterator__1nCurrent")
 private external fun _nCurrent(ptr: NativePointer): Int
@@ -564,4 +583,4 @@ private external fun _nGetRuleStatus(ptr: NativePointer): Int
 private external fun _nGetRuleStatuses(ptr: NativePointer): IntArray
 
 @ExternalSymbolName("org_jetbrains_skia_BreakIterator__1nSetText")
-private external fun _nSetText(ptr: NativePointer, textStr: InteropPointer, len: Int): NativePointer
+private external fun _nSetText(ptr: NativePointer, textStr: InteropPointer, len: Int, errorCode: InteropPointer): NativePointer

skiko/src/commonTest/kotlin/org/jetbrains/skia/BreakIteratorTests.kt

@@ -3,19 +3,25 @@ package org.jetbrains.skia
 import org.jetbrains.skiko.OS
 import org.jetbrains.skiko.hostOs
 import org.jetbrains.skiko.tests.SkipJsTarget
+import org.jetbrains.skiko.tests.SkipJvmTarget
+import org.jetbrains.skiko.tests.SkipNativeTarget
 import kotlin.test.Test
 import kotlin.test.assertContentEquals
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
+import kotlin.test.assertTrue
 
 private fun BreakIterator.asSequence() = generateSequence { next().let { n -> if (n == -1) null else n } }
 
+
 class BreakIteratorTests {
 
     @Test
+    @SkipJsTarget
     fun breakIteratorWordInstanceTest() {
         // Wasm and iOS builds of Skia do not include required data to implement those iterators,
         // see `third_party/externals/icu/flutter/README.md`.
-        if (hostOs == OS.JS || hostOs == OS.Ios)
+        if (hostOs == OS.Ios)
             return
 
         val boundary = BreakIterator.makeWordInstance()
@@ -31,16 +37,32 @@ class BreakIteratorTests {
     }
 
     @Test
+    @SkipNativeTarget
+    @SkipJvmTarget
+    fun breakIteratorSentenceFailsOnJsTest() {
+        // Wasm and iOS builds of Skia do not include required data to implement those iterators,
+        // see `third_party/externals/icu/flutter/README.md`.
+        // unfortunately js target does not check neither message nor the type of exception so here we can rely only on the fact there was exception
+        assertFailsWith<RuntimeException> {
+            BreakIterator.makeSentenceInstance()
+        }
+    }
+
+
+    @Test
+    @SkipJsTarget
     fun breakIteratorSentenceInstanceTest() {
         // Wasm and iOS builds of Skia do not include required data to implement those iterators,
         // see `third_party/externals/icu/flutter/README.md`.
-        if (hostOs == OS.JS)
-            return
+
         val boundary = BreakIterator.makeSentenceInstance()
-        boundary.setText("""
+
+        boundary.setText(
+            """
             Skiko (short for Skia for Kotlin) is the graphical library exposing significant part of Skia library APIs to Kotlin, along with the gluing code for rendering context.
             At the moment, Linux(x86_64 and arm64), Windows(x86_64) and macOS(x86_64 and arm64) builds for Kotlin/JVM are available.
-        """.trimIndent())
+        """.trimIndent()
+        )
 
         assertContentEquals(listOf(167, 287), boundary.asSequence().toList())
     }

skiko/src/jvmMain/cpp/common/BreakIterator.cc

@@ -11,25 +11,26 @@ extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_BreakIteratorKt_Break
 }
 
 extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_BreakIteratorKt__1nMake
-  (JNIEnv* env, jclass jclass, jint type, jstring localeStr) {
-    UErrorCode status = U_ZERO_ERROR;
+  (JNIEnv* env, jclass jclass, jint type, jstring localeStr, jintArray errorCode) {
+    UErrorCode errorCodes[1] = { U_ZERO_ERROR };
     UBreakIterator* instance;
     if (localeStr == nullptr)
-      instance = ubrk_open(static_cast<UBreakIteratorType>(type), uloc_getDefault(), nullptr, 0, &status);
+      instance = ubrk_open(static_cast<UBreakIteratorType>(type), uloc_getDefault(), nullptr, 0, errorCodes);
     else {
       SkString locale = skString(env, localeStr);
-      instance = ubrk_open(static_cast<UBreakIteratorType>(type), locale.c_str(), nullptr, 0, &status);
+      instance = ubrk_open(static_cast<UBreakIteratorType>(type), locale.c_str(), nullptr, 0, errorCodes);
     }
-    
-    if (U_FAILURE(status)) {
-      env->ThrowNew(java::lang::RuntimeException::cls, u_errorName(status));
+
+    env->SetIntArrayRegion(errorCode, 0, 1, reinterpret_cast<jint*>(errorCodes));
+
+    if (U_FAILURE(errorCodes[0])) {
       return 0;
     } else
       return reinterpret_cast<jlong>(instance);
 }
 
 extern "C" JNIEXPORT jint JNICALL Java_org_jetbrains_skia_BreakIteratorKt__1nClone
-  (JNIEnv* env, jclass jclass, jlong ptr) {
+  (JNIEnv* env, jclass jclass, jlong ptr, jintArray errorCode) {
     UBreakIterator* instance = reinterpret_cast<UBreakIterator*>(static_cast<uintptr_t>(ptr));
     UErrorCode status = U_ZERO_ERROR;
     UBreakIterator* clone = ubrk_clone(instance, &status);
@@ -109,16 +110,16 @@ extern "C" JNIEXPORT jintArray JNICALL Java_org_jetbrains_skia_BreakIteratorKt__
 }
 
 extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_BreakIteratorKt__1nSetText
-  (JNIEnv* env, jclass jclass, jlong ptr, jcharArray textArr, jint len) {
+  (JNIEnv* env, jclass jclass, jlong ptr, jcharArray textArr, jint len, jintArray errorCode) {
     UBreakIterator* instance = reinterpret_cast<UBreakIterator*>(static_cast<uintptr_t>(ptr));
 
     std::vector<jchar>* text = new std::vector<jchar>(len);
     env->GetCharArrayRegion(textArr, 0, len, text->data());
 
-    UErrorCode status = U_ZERO_ERROR;
-    ubrk_setText(instance, reinterpret_cast<UChar *>(text->data()), len, &status);
-    if (U_FAILURE(status))
-      env->ThrowNew(java::lang::RuntimeException::cls, u_errorName(status));
+    UErrorCode errorCodes[1] = { U_ZERO_ERROR };
+    ubrk_setText(instance, reinterpret_cast<UChar *>(text->data()), len, errorCodes);
+
+    env->SetIntArrayRegion(errorCode, 0, 1, reinterpret_cast<jint*>(errorCodes));
 
     return reinterpret_cast<jlong>(text);
 }
\ No newline at end of file

skiko/src/nativeJsMain/cpp/BreakIterator.cc

@@ -3,6 +3,7 @@
 
 #include "unicode/ubrk.h"
 #include "common.h"
+#include <iostream>
 
 static void deleteBreakIterator(UBreakIterator* instance) {
   ubrk_close(instance);
@@ -14,7 +15,7 @@ SKIKO_EXPORT KNativePointer org_jetbrains_skia_BreakIterator__1nGetFinalizer() {
 
 
 SKIKO_EXPORT KNativePointer org_jetbrains_skia_BreakIterator__1nMake
-  (KInt type, KInteropPointer localeStr) {
+  (KInt type, KInteropPointer localeStr, KInt* errorCode) {
     UErrorCode status = U_ZERO_ERROR;
     UBreakIterator* instance;
     if (localeStr == nullptr)
@@ -23,6 +24,7 @@ SKIKO_EXPORT KNativePointer org_jetbrains_skia_BreakIterator__1nMake
       instance = ubrk_open(static_cast<UBreakIteratorType>(type), skString(localeStr).c_str(), nullptr, 0, &status);
     }
 
+    errorCode[0] = status;
     if (U_FAILURE(status))
       return 0;
     else
@@ -31,10 +33,12 @@ SKIKO_EXPORT KNativePointer org_jetbrains_skia_BreakIterator__1nMake
      
 
 SKIKO_EXPORT KNativePointer org_jetbrains_skia_BreakIterator__1nClone
-  (KNativePointer ptr) {
+  (KNativePointer ptr, KInt* errorCode) {
     UBreakIterator* instance = reinterpret_cast<UBreakIterator*>(ptr);
     UErrorCode status = U_ZERO_ERROR;
     UBreakIterator* clone = ubrk_clone(instance, &status);
+
+    errorCode[0] = status;
     if (U_FAILURE(status)) {
       return 0;
     } else
@@ -116,11 +120,12 @@ SKIKO_EXPORT KInt* org_jetbrains_skia_BreakIterator__1nGetRuleStatuses
 
 
 SKIKO_EXPORT KNativePointer org_jetbrains_skia_BreakIterator__1nSetText
-  (KNativePointer ptr, KChar* textArr, KInt len) {
+  (KNativePointer ptr, KChar* textArr, KInt len, KInt* errorCode) {
     UBreakIterator* instance = reinterpret_cast<UBreakIterator*>(ptr);
     std::vector<UChar>* text = new std::vector<UChar>(textArr, textArr + len);
 
     UErrorCode status = U_ZERO_ERROR;
     ubrk_setText(instance, text->data(), len, &status);
+    errorCode[0] = status;
     return reinterpret_cast<KNativePointer>(text);
 }