Skip to content

[Rust][FFI] Wrap all extern "C" bodies in catch_unwind panic guards (crate-wide) #397

Description

@zlata-stefanovic-db

SDK

Rust

Description

None of the FFI extern "C" functions in rust/ffi/src wrap their bodies in std::panic::catch_unwind. If any of them panics (e.g. an unwrap/expect deep in a dependency, an allocation failure, or a slicing/bounds error), the panic unwinds across the FFI boundary into the C caller (Go via cgo, Java via JNI). Unwinding out of an extern "C" frame into non-Rust code is undefined behavior — at best an abort, at worst memory corruption — and it gives the caller no recoverable error.

This is preexisting and applies to every FFI entry point, not just the new dynamic-protobuf functions. Surfaced in review of #371 by @teodordelibasic-db:

none of the FFI functions wrap their bodies in catch_unwind. Worth a follow-up potentially to add panic guards crate-wide.

Proposed Solution

Wrap every #[no_mangle] pub extern "C" body in a catch_unwind guard so a panic is caught at the boundary and converted into the function's normal error channel instead of unwinding into C:

  • For functions that take a *mut CResult, write a non-retryable error into it (success = false, error_message set, is_retryable = false) and return the function's failure sentinel.
  • Use the correct per-signature sentinel on the caught-panic path: ptr::null_mut() / ptr::null() for pointer returns, false for bool, -1 for the i64 offset functions, etc.
  • Prefer a single shared helper (a small wrapper fn or macro) applied uniformly across the crate so the pattern is consistent and hard to forget on new functions, rather than hand-writing catch_unwind in each function.
  • Note that catch_unwind requires the closure to be UnwindSafe; raw pointers are, but any captured &mut or interior-mutable state may need AssertUnwindSafe with a brief justification.

Add tests that force a panic in a guarded function (e.g. via a deliberately panicking HeadersProvider callback or malformed input) and assert it returns the failure sentinel + a populated CResult rather than aborting.

Additional Context

Follow-up from #371; tracked separately from that PR since it is preexisting and crate-wide.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestrefactoringRefactoring & tech debt

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions