apache / apache/hudi-rs

cpp: guard FFI entry points with catch_unwind so panics don't abort the host process

Open
#636 0 comments 0 reactions 1 assignee Claimed by @codope View on GitHub
Dominant language
Rust
Stars
279
Forks
67
Avg merge
3d 1h
Merged PRs (30d)
40

Description

## Summary

The C++ bindings in `cpp/src/lib.rs` call into Rust code that can `panic!` (or hit `todo!()` / `unimplemented!()`). When a panic unwinds across the `cxx` FFI boundary into the C++ caller, the result is undefined behavior / a process abort rather than a catchable error.

For engines that embed the C++ library in a long-lived, multi-tenant worker (e.g. Presto native / Velox "Prestissimo", where one process runs many concurrent queries — see apache/hudi#18308), a single unsupported input would crash the entire worker and take down every co-running query, instead of failing just that one read.

## Where panics can originate

Reachable from a file-slice read:

- `crates/core/src/avro_to_arrow/schema.rs` and `arrow_array_reader.rs` — `todo!()` / `unimplemented!()` for Map, RunEndEncoded, View, several temporal/dictionary types, and `AvroSchema::Ref` (e.g. a table with a `map<>` column, read via Avro log blocks).
- `crates/core/src/file_group/log_file/scanner.rs` — internal `panic!` unwraps on unexpected scan-result variants.

Any of these can be hit legitimately by real-world tables and would propagate as a panic through the C++ entry points in `cpp/src/lib.rs`.

## Proposal

Guard each `extern "Rust"` entry point in `cpp/src/lib.rs` with `std::panic::catch_unwind` (`AssertUnwindSafe` as needed) and convert a caught panic into the existing `Result<_, String>` error, so it surfaces to C++ as a normal `rust::Error` that the caller can catch and handle (e.g. fall back to another reader).

This is a small, isolated change to the FFI shim and keeps the panic-to-error policy at the boundary rather than requiring every internal call site to become fallible.

## Why now

This is a prerequisite for safely embedding the C++ library in a shared query-engine worker. Tracking it separately from #18308 so it can be picked up independently.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.