avantgardnerio opened a new pull request, #22742: URL: https://github.com/apache/datafusion/pull/22742
## Which issue does this PR close? - Closes #. Follow-up defect fix in the allocator-level accounting framework added by #22626. ## Rationale for this change `AccountingAllocator::realloc` (introduced in #22626) called `inner.realloc` first — which frees the caller's old pointer on success — then called `track`, which can `panic_any` on overdraft. The unwind starts with the caller's `Vec` (or similar growing container) still holding the now-freed old pointer; the next `Drop` produces glibc's `double free or corruption (out)` and SIGABRT, masking the underlying untracked-allocation panic the framework was trying to surface. The same hazard does not apply to `alloc` / `alloc_zeroed` / `dealloc`: - `alloc` / `alloc_zeroed` — the caller has no preexisting pointer to be invalidated; panicking after the inner alloc just leaks the new allocation, which is fine on the kill path. - `dealloc` — credits the bank, never panics. Only `realloc` has a live caller-side pointer that gets invalidated by the inner operation before the panic decision. ## What changes are included in this PR? Reorder `AccountingAllocator::realloc` to account first, forward to `inner.realloc` second: - `track(delta)` first — on overdraft we panic with the caller's pointer still valid; unwind drops the live container cleanly, no abort. - If `inner.realloc` returns null after a successful track, refund the delta so the bank stays consistent with what actually got allocated. ## Are these changes tested? Manually verified using a `GroupedHashAggregateStream` + `List<Utf8>` group-key reproducer (routes through `GroupValuesRows::intern` → `RowConverter::append` → `Vec::resize` → `__rust_realloc`, which is one of the untracked-allocation sites this framework is meant to catch): - before: exit 101, `double free or corruption (out)`, signal 6 SIGABRT - after: exit 1, clean `allocator overdraft: account balance at panic = -1.7 MB` at the same `RowConverter::append` stack frame A regression test for "panic from inside `realloc` does not double-free" would require constructing the precise realloc-mid-grow scenario plus a `catch_unwind` harness; deferred as follow-up. ## Are there any user-facing changes? No. Confined to `sqllogictest`'s `memory-accounting` feature, which is internal CI tooling. -- 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]
