[PR] fix: drop JNI GlobalRef before detaching thread in memory pool errors [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new pull request, #3832:
URL: https://github.com/apache/datafusion-comet/pull/3832

   ## Which issue does this PR close?
   
   Closes #2470.
   
   ## Rationale for this change
   
   When running with reduced off-heap memory, JNI `GlobalRef` objects inside `JavaException` errors can be dropped on tokio worker threads that are not attached to the JVM. This triggers warnings from the `jni` crate (`"Dropping a GlobalRef in a detached thread"`) and causes expensive temporary attach/detach cycles.
   
   The root cause: memory pool methods (`acquire_memory`/`release_memory`) call JNI via a temporary `AttachGuard`. If the JNI call throws a Java exception, a `CometError::JavaException` is created containing a `GlobalRef` to the throwable. When the method returns, the `AttachGuard` drops and detaches the thread — but the `GlobalRef` inside the error outlives it. As the error propagates through DataFusion on the now-detached tokio thread and is eventually converted to `DataFusionError::Execution(String)`, the `GlobalRef` is dropped on the detached thread, triggering the warning.
   
   ## What changes are included in this PR?
   
   - Add `CometError::drop_throwable()` which converts `JavaException` errors (containing a `GlobalRef`) into string-only `Internal` errors, ensuring the `GlobalRef` is dropped while the thread is still JVM-attached.
   - Apply `.map_err(CometError::drop_throwable)` in all four memory pool JNI methods: `acquire_from_spark`, `release_to_spark` (unified pool), `acquire`, `release` (fair pool).
   
   The `GlobalRef` is safe to drop early here because these errors always propagate through `DataFusionError::Execution(String)` which stringifies the error anyway — the throwable reference is never used to re-throw the original Java exception from this path.
   
   ## How are these changes tested?
   
   This is difficult to test in a unit test since it requires a full Spark executor environment with memory pressure on tokio worker threads. The fix is verified by code inspection: `map_err` executes while the `AttachGuard` (`env`) is still in scope, so the `GlobalRef` is released on an attached thread. Clippy passes cleanly.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix: drop JNI GlobalRef before detaching thread in memory pool errors [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove closed pull request #3832: fix: drop JNI GlobalRef before detaching thread in memory pool errors
URL: https://github.com/apache/datafusion-comet/pull/3832


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org