andygrove commented on issue #5212: URL: https://github.com/apache/datafusion-comet/issues/5212#issuecomment-5159291390
Additional finding from reviewing the fix for position 2 — the sibling registry has the same defect: ### 20. `THREAD_MEMORY_POOLS` entries leak on the same `createPlan` failure path `native/core/src/execution/jni_api.rs:143-149` (registration), `:957-965` (explicit unregister), call site `:494` `register_memory_pool(rust_thread_id, id, Arc::clone(&session.runtime_env().memory_pool))` runs at `:494`, but fallible work still follows inside `createPlan` — the `jni_new_global_ref!` for `task_context` at `:511`, and anything added between. If that fails, the `(thread_id, context_id)` entry is stranded in a map that nothing prunes, holding an `Arc<dyn MemoryPool>`. For the unified pools that `Arc` holds the JNI global ref to `CometTaskMemoryManager`, so it pins `TaskMemoryManager`/`TaskContext` exactly as in position 2. Context ids are unique per plan, so it is unbounded. Secondary effect: `total_reserved_for_thread` (`:172`) then sums dead pools, so the tracing memory numbers drift upward over the executor's lifetime. Scope is narrower than position 2 — the registration only happens when `spark.comet.tracing.enabled` is set (`:489`). The fix is the same guard shape introduced in #5217: a drop-only field on `ExecutionContext` whose `Drop` performs today's `unregister_and_total` body, leaving the "emit remaining total" log in `releasePlan`. I left it out of #5217 to keep that diff scoped to one registry, so after it merges one of the two per-plan global registries is RAII-managed and the other is not. -- 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]
