github-actions[bot] commented on code in PR #69889:
URL: https://github.com/apache/doris/pull/69889#discussion_r4422443606


##########
be/src/util/jni-util.cpp:
##########
@@ -42,6 +45,66 @@ jclass Env::jni_util_cl_ = nullptr;
 jmethodID Env::throwable_to_string_id_ = nullptr;
 jmethodID Env::throwable_to_stack_trace_id_ = nullptr;
 
+template <RefType Ref>
+Status String<Ref>::new_string_utf8(JNIEnv* env, std::string_view utf_chars, 
String<Ref>* result) {
+    DCHECK(result->uninitialized());
+    if constexpr (Ref == Global) {
+        String<Local> local_result;
+        RETURN_IF_ERROR(String<Local>::new_string_utf8(env, utf_chars, 
&local_result));
+        RETURN_IF_ERROR(local_to_global_ref(env, local_result, result));
+    } else {
+        static_assert(Ref == Local);
+        // JNI lengths are signed 32-bit integers; reject before narrowing or 
allocating.
+        if (utf_chars.size() > 
static_cast<size_t>(std::numeric_limits<jsize>::max())) {
+            return Status::InvalidArgument("UTF-8 string exceeds JNI 
allocation bound");
+        }
+        // One UTF-8 byte per UTF-16 unit is an upper bound: e.g. an emoji 
uses 4 bytes, 2 units.
+        // Keep one unit for empty input so both simdutf and JNI receive a 
non-null buffer.
+        DorisVector<char16_t> utf16(std::max<size_t>(utf_chars.size(), 1));
+        // A default-constructed empty string_view has a null data pointer.
+        const auto* input = utf_chars.empty() ? "" : utf_chars.data();
+        const auto converted =
+                simdutf::convert_utf8_to_utf16_with_errors(input, 
utf_chars.size(), utf16.data());
+        // Reject malformed UTF-8 (e.g. a truncated emoji) instead of silently 
changing SQL.
+        if (converted.error != simdutf::SUCCESS) {
+            return Status::InvalidArgument("Invalid UTF-8 in JNI string at 
byte {}",
+                                           converted.count);
+        }
+        result->_obj = env->NewString(reinterpret_cast<const 
jchar*>(utf16.data()),
+                                      static_cast<jsize>(converted.count));

Review Comment:
   [P1] Use a callback-safe allocator for the UTF-8 conversion scratch buffers. 
`_reserve_spill` and `_update_spill_accounting` call `get_string_utf8`, while 
`_get_spill_directories` and `_throw_java_io_exception` can reach 
`new_string_utf8`; Paimon invokes these through its plain 
`doris-paimon-compaction` executor. `DorisVector`'s default allocator runs 
`memory_check()` through `thread_context()`, but 
`JvmLauncher::attach_current_thread` only attaches JNI and these callbacks 
never use `SCOPED_ATTACH_TASK`/`with_resource_context`. A valid spill callback 
can therefore throw the no-thread-context FatalError before updating 
accounting, and the C++ exception cannot safely cross JNI. Use a 
callback-safe/non-tracking temporary buffer or attach the proper resource 
context at callback entry.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to