[felipepessoto]

What changes are proposed in this pull request?

Java callback failures currently become text in checkException and return as a new GlutenException. This loses the original exception type, cause chain, and Spark/Delta metadata. This patch preserves the same Java throwable object at the JNI return boundary instead. Existing public iterator Exception wrappers remain unchanged; inspect their cause chain rather than assuming the original is the top-level exception.

• Ownership: gluten::JavaException holds a shared JNI global reference across native copies, std::exception_ptr, and thread handoffs. Cleanup uses the current thread's JNI environment and detaches only threads it attached.
• Propagation: checkException captures the throwable; JNI_METHOD_END restores it through direct or standard nested exceptions. Successful native propagation keeps JNI clear until the return boundary. Native-only failures retain the existing GlutenException fallback.
• Failure handling: diagnostics remain best-effort. Failed carrier capture restores or retains pending Java state only inside a guarded JNI entry on the current thread; native-only workers are cleared before the native failure propagates. Failed JNI restoration uses the existing fallback only when nothing is pending, and logs fallback failure.

Scope: Backend-neutral JNI transport, layer 1 of 3. Velox iterator and memory-callback integration follow in #13561 and #13563; #13133 is the combined reference. This layer alone does not protect the full Velox driver path, translate native errors, or merge Java/C++ stack traces.

How was this patch tested?

• 14 native tests passed, covering reference lifetime, thread cleanup, nesting, allocation failures, worker pending-state cleanup, and nonallocating entry tracking. Worker state/global-reference capture regressions fail before the fix.
• 14 JUnit tests passed through normal Maven Surefire, retaining the original identity/metadata and restoration controls while adding reused-worker capture failure, successful cross-thread carrier transfer, and nested Java reentry. Real-JVM scalar-allocation probes also verified worker reuse and guarded Java identity; cross-DSO scope checks passed. The existing same-job jni-test Maven path is unchanged.
• C++/Java formatting and license-header checks passed.

These are focused core/JNI checks; this PR does not claim end-to-end Velox or Delta coverage.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: GitHub Copilot 1.0.102-0.unstable.r40949665530.gab1eef5 (model: gpt-6-astra)

[felipepessoto]

Hi @zhztheplayer @marin-ma @zhouyuan, could you review #13560? It’s the core-only split from #13133, covering Java throwable preservation and JNI reference lifetime. The Velox integration is in separate PRs, and CI is green. Thanks!

[Copilot]

🟡 Changes recommended

Carrier allocation failure can discard the original Java throwable and replace it with a bad_alloc-based GlutenException; construction needs a restoration guard and targeted failure coverage.

1 open issue

• Restore the original throwable if carrier allocation fails · New

What changed in this PR

This PR adds backend-neutral transport for original Java throwables across JNI boundaries. It is the core layer of the split implementation; backend-specific propagation and hybrid Java/C++ stack traces remain outside its scope.

Changes:

• Add JavaException with shared global-reference ownership and thread-aware cleanup.
• Update JNI exception handling to restore direct and standard nested Java carriers while retaining native-error fallback.
• Register focused lifetime and failure-path tests, isolate allocator fault injection, and document the transport contract.

File	Description
docs/​developers/​NewToGluten.md	Document exception preservation and scope limits.
cpp/​core/​tests/​utils/​TestJniEnvironment.h	Add a mock JNI environment for ownership and failure tests.
cpp/​core/​tests/​JavaExceptionTest.cc	Test lifetime, thread cleanup, nesting, and JNI failures.
cpp/​core/​tests/​JavaExceptionAllocationTest.cc	Test diagnostic allocation failure in isolation.
cpp/​core/​tests/​CMakeLists.txt	Register both exception test executables.
cpp/​core/​jni/​JniError.h	Restore original throwables at JNI exit boundaries.
cpp/​core/​jni/​JniCommon.h	Delegate callback exception capture to JavaException.
cpp/​core/​jni/​JavaException.h	Define the throwable carrier interface.
cpp/​core/​jni/​JavaException.cc	Implement ownership, diagnostics, cleanup, and restoration.
cpp/​core/​CMakeLists.txt	Include the carrier in the core library build.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

The review reports no active issues with the JNI throwable-preservation change.

No open issues 🎉

1 resolved since last review

• Restore the original throwable if carrier allocation fails

_{Review effort: Balanced}

[zhztheplayer]

Thanks. Can we have light Java-side test cases to cover the changes?

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

• activeByDefault profiles are only active when no other profiles are explicitly enabled. In this… · New
• The jni_test change detector matches pom\\.xml at the repository root, which will trigger the… · New
• The jni_test change detector matches pom\\.xml at the repository root, which will trigger the… · New

3 resolved since last review

• JNI_METHOD_END now references gluten::JavaException, but this header doesn’t show any include…
• This test binary overrides global scalar operator new/delete but not operator new[]/delete[]…
• e.throwToJava(env) is invoked unconditionally. If a newer JNI exception is already pending when…

_{Review effort: Lite}

[Copilot]

🟢 Approval recommended

The supplied review evidence reports no active issues with the backend-neutral Java throwable transport changes.

3 open issues

• activeByDefault profiles are only active when no other profiles are explicitly enabled. In this…
• The jni_test change detector matches pom\\.xml at the repository root, which will trigger the…
• The jni_test change detector matches pom\\.xml at the repository root, which will trigger the…

_{Review effort: Balanced}

[rdtr]

Thanks for splitting this out, @felipepessoto, and for the Java-side tests, including the already-pending cases and the named local capacity in the latest push. I read the diff and the surrounding callers, and I also looked at #13561 and #13563 where they use this API. This is from reading the code only, I did not build or run anything. The reference handling reads correct to me. The global ref is deleted once by the last shared_ptr owner, copies and exception_ptr handoffs share it, and the detach only happens on a thread the destructor attached itself. Three inline comments, none of them blocking from my side. The first is a risk on attached pool threads when memory is short, the second is a question about the doc and ClosableIterator, and the third is a small doc suggestion.

[rdtr]

Risk, only when memory is short. When the JavaException can't be constructed here (make_shared<State> throws or NewGlobalRef returns null), this line puts the original throwable back as a pending exception with env->Throw(throwable) and rethrows the C++ error.

Normally the thread entered native code through a JNI method, so the C++ error unwinds to JNI_METHOD_END, which sees the pending exception, leaves it alone and returns to Java. For example on the input iterator path:

1. ColumnarBatchOutIterator.nativeHasNext enters native code under JNI_METHOD_START.
   

   gluten/cpp/core/jni/JniWrapper.cc

   Lines 450 to 462 in bb6b836

   	JNIEXPORT jboolean JNICALL Java_org_apache_gluten_vectorized_ColumnarBatchOutIterator_nativeHasNext( // NOLINT
   	JNIEnv* env,
   	jobject wrapper,
   	jlong iterHandle) {
   	JNI_METHOD_START
   	auto iter = ObjectStore::retrieve<ResultIterator>(iterHandle);
   	if (iter == nullptr) {
   	std::string errorMessage =
   	"When hasNext() is called on a closed iterator, an exception is thrown. To prevent this, consider using the protectInvocationFlow() method when creating the iterator in scala side. This will allow the hasNext() method to be called multiple times without issue.";
   	throw GlutenException(errorMessage);
   	}
   	return iter->hasNext();
   	JNI_METHOD_END(false)

2. Velox runs the task serially on that same thread, and the input operator calls RowVectorStream::hasNext.
   

   gluten/cpp/velox/compute/WholeStageResultIterator.cc

   Lines 124 to 138 in bb6b836

   	const bool serialExecution = true;
   	facebook::velox::exec::CursorParameters params;
   	params.planNode = planNode;
   	params.destination = 0;
   	params.maxDrivers = 1;
   	params.queryCtx = createNewVeloxQueryCtx();
   	params.executionStrategy = velox::core::ExecutionStrategy::kUngrouped;
   	params.groupedExecutionLeafNodeIds = std::move(emptySet);
   	params.numSplitGroups = 1;
   	params.spillDirectory = spillDir;
   	params.serialExecution = serialExecution;
   	params.copyResult = false;
   	params.outputPool = memoryManager_->getLeafMemoryPool();
   	cursor_ = velox::exec::TaskCursor::create(params);

   
   

   gluten/cpp/velox/operators/plannodes/RowVectorStream.cc

   Lines 51 to 78 in bb6b836

   	bool RowVectorStream::hasNext() {
   	if (finished_) {
   	return false;
   	}
   	VELOX_DCHECK_NOT_NULL(iterator_);
   	bool hasNext;
   	{
   	// We are leaving Velox task execution and are probably entering Spark code through JNI. Suspend the current
   	// driver to make the current task open to spilling.
   	//
   	// When a task is getting spilled, it should have been suspended so has zero running threads, otherwise there's
   	// possibility that this spill call hangs. See https://github.com/apache/gluten/issues/7243.
   	// As of now, non-zero running threads usually happens when:
   	// 1. Task A spills task B;
   	// 2. Task A tries to grow buffers created by task B, during which spill is requested on task A again.
   	const facebook::velox::exec::DriverThreadContext* driverThreadCtx = facebook::velox::exec::driverThreadContext();
   	VELOX_CHECK_NOT_NULL(
   	driverThreadCtx,
   	"ExternalStreamDataSource::next() is not called "
   	"from a driver thread");
   	SuspendedSection ss(driverThreadCtx->driverCtx()->driver);
   	hasNext = iterator_->hasNext();
   	}
   	if (!hasNext) {
   	finished_ = true;
   	}
   	return hasNext;

3. JniColumnarBatchIterator::nextInternal calls the Java iterator and then checkException, which is JavaException::check.
   

   gluten/cpp/core/jni/JniCommon.cc

   Lines 300 to 311 in bb6b836

   	std::shared_ptr<gluten::ColumnarBatch> gluten::JniColumnarBatchIterator::nextInternal() const {
   	JNIEnv* env = nullptr;
   	attachCurrentThreadAsDaemonOrThrow(vm_, &env);
   	if (!env->CallBooleanMethod(jColumnarBatchItr_, serializedColumnarBatchIteratorHasNext_)) {
   	checkException(env);
   	return nullptr; // stream ended
   	}
   	checkException(env);
   	jlong handle = env->CallLongMethod(jColumnarBatchItr_, serializedColumnarBatchIteratorNext_);
   	checkException(env);
   	return ObjectStore::retrieve<ColumnarBatch>(handle);
   	}

   
   

   gluten/cpp/core/jni/JniCommon.h

   Lines 56 to 57 in bb6b836

   	static inline void checkException(JNIEnv* env) {
   	gluten::JavaException::check(env);

4. Construction fails and this line restores the throwable.
   

   gluten/cpp/core/jni/JavaException.cc

   Lines 168 to 176 in bb6b836

   	auto error = [env, throwable]() {
   	try {
   	return JavaException(env, throwable);
   	} catch (...) {
   	if (!env->ExceptionCheck() && env->Throw(throwable) != JNI_OK) {
   	LOG(ERROR) << "Unable to restore original Java exception";
   	}
   	throw;
   	}

5. The C++ error unwinds to the std::exception branch of JNI_METHOD_END. env->ExceptionCheck() is true there, so it neither restores a carrier nor throws GlutenException, and nativeHasNext returns false.
   

   gluten/cpp/core/jni/JniError.h

   Lines 43 to 51 in bb6b836

   	catch (const std::exception& e) { \
   	if (!env->ExceptionCheck()) { \
   	gluten::JavaException::throwNestedToJava(env, e); \
   	} \
   	if (!env->ExceptionCheck() && \
   	env->ThrowNew(gluten::getJniErrorState()->glutenExceptionClass(), e.what()) != JNI_OK) { \
   	LOG(ERROR) << "Unable to throw GlutenException at JNI boundary"; \
   	} \
   	return fallback_expr; \

The JVM then throws the original in Java when the native method returns, which also clears it from the thread.

However, when the thread was started by Gluten's C++ code or by Velox, there is no Java frame below it to return to. Examples are the folly pools in VeloxBackend (spill, IO, broadcast build).

gluten/cpp/velox/compute/VeloxBackend.cc

Lines 250 to 274 in bb6b836

	const auto spillThreadNum = backendConf_->get<uint32_t>(kSpillThreadNum, kSpillThreadNumDefaultValue);
	if (spillThreadNum > 0) {
	spillExecutor_ = std::make_unique<folly::CPUThreadPoolExecutor>(spillThreadNum);
	}
	const auto ioThreads = backendConf_->get<int32_t>(kVeloxIOThreads, numTaskSlotsPerExecutor);
	GLUTEN_CHECK(
	ioThreads >= 0,
	kVeloxIOThreads + " was set to negative number " + std::to_string(ioThreads) + ", this should not happen.");
	if (ioThreads > 0) {
	ioExecutor_ =
	std::make_unique<folly::CPUThreadPoolExecutor>(ioThreads, folly::CPUThreadPoolExecutor::makeLifoSemQueue());
	}
	const int32_t broadcastHashTableBuildThreads = std::max<int32_t>(1, [&]() {
	if (!backendConf_->valueExists(kBroadcastHashTableBuildThreads)) {
	LOG(WARNING) << kBroadcastHashTableBuildThreads << " is not set. Falling back to " << kNumTaskSlotsPerExecutor
	<< " (" << numTaskSlotsPerExecutor << ").";
	return numTaskSlotsPerExecutor;
	}
	return backendConf_->get<int32_t>(kBroadcastHashTableBuildThreads).value();
	}());
	if (broadcastHashTableBuildThreads > 1) {
	broadcastHashTableBuildExecutor_ = std::make_unique<folly::CPUThreadPoolExecutor>(broadcastHashTableBuildThreads);
	}

Jobs submitted through HookedExecutor attach the thread before they run, and SparkThreadInitializer::destroy deliberately doesn't detach afterwards, so the thread stays attached for later jobs.

gluten/cpp/velox/compute/VeloxRuntime.cc

Lines 167 to 205 in bb6b836

	folly::Func wrap(folly::Func func, int8_t priority) {
	auto initializer = initializer_;
	auto state = state_;
	const auto taskId = state->nextTaskId.fetch_add(1, std::memory_order_relaxed);
	const auto taskName = name_ + std::to_string(taskId);
	if (debug_) {
	TaskInfo info{
	.enqueueTime = std::chrono::steady_clock::now(),
	.priority = priority,
	.submitStacktrace = velox::process::StackTrace().toString()};
	std::lock_guard<std::mutex> lock(state->taskMutex);
	state->inFlightTasks[taskId] = std::move(info);
	}
	const auto debug = debug_;
	return [func = std::move(func), initializer, state, debug, taskId, taskName]() mutable {
	auto markDone = folly::makeGuard([&] {
	if (debug) {
	std::lock_guard<std::mutex> lock(state->taskMutex);
	state->inFlightTasks.erase(taskId);
	}
	if (state->inFlight.fetch_sub(1, std::memory_order_acq_rel) == 1) {
	std::lock_guard<std::mutex> lock(state->mutex);
	state->cv.notify_all();
	}
	});
	GLUTEN_CHECK(initializer != nullptr, "ThreadInitializer is null.");
	initializer->initialize(taskName);
	auto invokeDestroyer = folly::makeGuard([&] { initializer->destroy(taskName); });
	// Destroy the submitted callable and all of its captures before
	// decrementing inFlight_. Some async tasks capture AsyncLoadHolder,
	// which keeps a MemoryPool alive until the callable itself is
	// destroyed. If we decrement inFlight_ first, HookedExecutor can
	// appear drained and let VeloxRuntime teardown proceed while the
	// holder is still alive, causing MemoryManager destruction to race
	// with outstanding task-owned resources.
	auto localFunc = std::move(func);
	localFunc();
	};
	}

gluten/cpp/core/jni/JniCommon.h

Lines 611 to 631 in bb6b836

	void initialize(const std::string& threadName) override {
	JNIEnv* env;
	attachCurrentThreadAsDaemonOrThrow(vm_, &env);
	jstring jThreadName = env->NewStringUTF(threadName.c_str());
	env->CallVoidMethod(jInitializerGlobalRef_, initializeMethod(env), jThreadName);
	env->DeleteLocalRef(jThreadName);
	checkException(env);
	}
	void destroy(const std::string& threadName) override {
	// IMPORTANT: Do not call vm_.DetachCurrentThread here, otherwise Java side thread
	// object might be dereferenced and garbage-collected, to break the reuse of thread
	// resources.
	JNIEnv* env;
	attachCurrentThreadAsDaemonOrThrow(vm_, &env);
	jstring jThreadName = env->NewStringUTF(threadName.c_str());
	env->CallVoidMethod(jInitializerGlobalRef_, destroyMethod(env), jThreadName);
	env->DeleteLocalRef(jThreadName);
	checkException(env);
	}

The SparkAllocationListener constructor comment already expects calls from IO threads.

gluten/cpp/core/jni/JniCommon.h

Line 403 in bb6b836

// Resolve JNI symbols on the creator thread to avoid first-touch initialization from IO threads.

On such a thread the C++ error is caught somewhere in C++, nothing clears the pending exception, and the thread goes back to its pool with it. The next job on that thread, possibly for a different task, makes a JNI call with an exception pending, and its checkException reports the old throwable as its own error. If NewGlobalRef fails with a pending OutOfMemoryError, line 172 keeps that one pending instead, with the same effect. Before this PR checkException always cleared the exception, so this is new.

Could the restore happen only when a JNI entry is on the stack? For example JNI_METHOD_START could hold a small RAII guard that bumps a thread_local depth counter, and check() would restore only when the depth is above zero and clear otherwise. Pool threads that attached themselves have depth zero, so they keep the old clearing behavior, and a JNI method that doesn't use the macro would only lose the original in this rare case. If that's too much for this PR, always clearing here as checkException did before would also avoid the carry-over. JNI_METHOD_END would then report the C++ error as a GlutenException, so only the original throwable is lost, and only when memory is short.

[felipepessoto]

Added a nonallocating JniEntryGuard to JNI_METHOD_START. Failed carrier construction now restores or retains pending Java state only when the current thread has a guarded JNI entry. Otherwise it clears the pending exception before propagating the native failure, without detaching the worker. Successfully captured carriers still use shared global references and remain transferable across threads.

The regressions reproduce both initial-state and global-reference allocation failures on native workers. They now verify that the same attached worker can execute another Java callback, while guarded JNI entries still preserve the original or newer pending exception. Nested Java reentry and cross-library scope checks also pass.

[rdtr]

Could describe get a short comment on where its result ends up? Reading only this file, the message looks diagnostic-only, since JNI_METHOD_END rethrows the original and only falls back to what() when that fails. With the pinned Velox a throw inside an operator call is caught by CALL_OPERATOR and rethrown as VELOX_FAIL("Operator::{} failed for [...]: {}", ..., e.what()).
https://github.com/IBM/velox/blob/6deac80271569277bf54b6d04547620c17438348/velox/exec/Driver.cpp#L74-L84
https://github.com/IBM/velox/blob/6deac80271569277bf54b6d04547620c17438348/velox/exec/Driver.cpp#L413-L414

#13561 and #13563 wrap the input-iterator and memory callbacks so the object survives that, but the jol:/jni: spill filesystem callbacks in JniFileSystem.cc, used when spillFileSystem=heap-over-local is set, aren't wrapped in any of the three PRs. On that path this string stays the only part of the Java exception that reaches Spark. The destructors there also log only e.what().

gluten/cpp/velox/jni/JniFileSystem.cc

Lines 83 to 84 in bb6b836

	} catch (const std::exception& e) {
	LOG(WARNING) << "Error closing jni read file " << e.what();

gluten/cpp/velox/jni/JniFileSystem.cc

Lines 164 to 165 in bb6b836

	} catch (const std::exception& e) {
	LOG(WARNING) << "Error closing jni write file " << e.what();

A line like "Used by what(). Paths that can't carry the JavaException itself, such as Velox operator wrapping and destructor logging, only see this text." would explain why it is built eagerly and why failures fall back to a placeholder.

[felipepessoto]

Added a short comment above describe explaining that lossy native wrappers and destructor logging may have only what(), not the carrier. Eager best-effort formatting and the placeholder fallback are unchanged. This core PR does not add carrier wrapping to the jni:/jol: filesystem callbacks.

[rdtr]

Question. This says JNI_METHOD_END restores the same object, and the JNI boundary does. On the main iterator path the object is wrapped again right after. ClosableIterator.hasNext() and next() catch Exception and call translateException, and ColumnarBatchOutIterator.translateException returns new GlutenException(e) unless the message matches a schema error.

gluten/gluten-core/src/main/java/org/apache/gluten/iterator/ClosableIterator.java

Lines 37 to 38 in bb6b836

	} catch (Exception e) {
	throw translateException(e);

gluten/gluten-core/src/main/java/org/apache/gluten/iterator/ClosableIterator.java

Lines 49 to 50 in bb6b836

	} catch (Exception e) {
	throw translateException(e);

gluten/gluten-arrow/src/main/java/org/apache/gluten/vectorized/ColumnarBatchOutIterator.java

Lines 147 to 157 in bb6b836

	protected RuntimeException translateException(Exception e) {
	String msg = findFirstNonNullMessage(e);
	if (msg != null) {
	RuntimeException schemaEx = translateToSchemaException(msg);
	if (schemaEx != null) {
	schemaEx.initCause(e);
	return schemaEx;
	}
	}
	return new GlutenException(e);
	}

Code that matches on the top-level class or SparkThrowable sees GlutenException, with the original only as the cause. The tests in #13561 show the same thing. The identity checks call hasNext0()/next0() directly, and nested native iterators retain the actual Java cause expects GlutenException -> GlutenException -> original through outer.hasNext().

Neither #13561 nor #13563 touches ClosableIterator. Is the wrapping intended to stay? If so, I think the doc should say callers of hasNext()/next() get the original as a cause. If not, this seems like the place to stop wrapping an exception that came from a Java callback.

[felipepessoto]

Yes, the existing Java-side wrapping is intended to stay. Updated the documentation and PR description to make the distinction explicit: the same throwable is restored at the JNI return boundary, while public hasNext()/next() may wrap an Exception in GlutenException or the existing schema-error translation, preserving the original in the cause chain.

Callers therefore cannot assume the original is the top-level class or SparkThrowable. ClosableIterator and ColumnarBatchOutIterator behavior is unchanged; the preservation guarantee is not a change to their wrapping policy.

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the JNI throwable-transport changes.

No open issues 🎉

3 resolved since last review

• activeByDefault profiles are only active when no other profiles are explicitly enabled. In this…
• The jni_test change detector matches pom\\.xml at the repository root, which will trigger the…
• The jni_test change detector matches pom\\.xml at the repository root, which will trigger the…

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Cross-thread JNI cleanup and allocation-failure recovery alter the shared JNI contract, requiring human validation of native/JVM CI evidence, including cross-library entry tracking, beyond static review.

No open issues 🎉

_{Review effort: Balanced}

[felipepessoto]

Thanks @rdtr. I sent the changes and CI is green