Skip to content

fix(sdk/jni): guard finalizeScannerPairing against panics across FFI#435

Open
dsmfa10 wants to merge 1 commit into
mainfrom
fix/sdk-jni-finalize-pairing-panic-guard
Open

fix(sdk/jni): guard finalizeScannerPairing against panics across FFI#435
dsmfa10 wants to merge 1 commit into
mainfrom
fix/sdk-jni-finalize-pairing-panic-guard

Conversation

@dsmfa10
Copy link
Copy Markdown
Collaborator

@dsmfa10 dsmfa10 commented Jun 3, 2026

Problem

A Rust panic must never unwind across an extern "system" (FFI) boundary — doing
so is undefined behavior and aborts the JVM. The module's safety contract states
every JNI export wraps its body in a jni_catch_unwind_* helper. Every export in
ble_events.rs and unified_protobuf_bridge.rs follows this — except
Java_..._finalizeScannerPairing, which called env.get_string(..) and
rt.block_on(orchestrator.finalize_scanner_pairing_by_address(..)) directly:

// src/jni/ble_events.rs (before)
pub extern "system" fn Java_..._finalizeScannerPairing(env, _clazz, ble_address_jstr) -> jboolean {
    let mut env = match unsafe { jni::JNIEnv::from_raw(...) } { ... };
    let ble_address: String = match env.get_string(&ble_address_ref) { ... };   // can panic
    match rt.block_on(orchestrator.finalize_scanner_pairing_by_address(&ble_address)) { ... }  // can panic
}

A panic in the orchestrator/runtime/JNI would cross the FFI boundary.

Fix

Wrap the body in jni_catch_unwind_jboolean (returns JNI_FALSE/0 on panic),
mirroring the established pattern used by the other exports:

pub extern "system" fn Java_..._finalizeScannerPairing(env, _clazz, ble_address_jstr) -> jboolean {
    crate::jni::bridge_utils::jni_catch_unwind_jboolean(
        "finalizeScannerPairing",
        std::panic::AssertUnwindSafe(move || {
            // ...original body unchanged...
        }),
    )
}

Verification

Cannot cargo check here (android+jni cfg-gated). The wrapper is the same helper

  • AssertUnwindSafe(move || ...) form used by ~15 sibling exports in the same
    file; the captured params are Copy raw JNI pointers (UnwindSafe). Reviewer
    should build the Android target
    — rustfmt will also normalize the wrapped
    body's indentation there.

CI gates & coverage

Full verification for this PR runs in CI — Rust (cargo fmt --check,
clippy -D warnings, workspace tests), Frontend, Android Unit Tests,
Coverage, SPDX headers, CodeQL (see the PR's Checks tab). The local
check noted above is a subset; the broader mandated gates (full workspace test
suite, codegen/scan, Android, Frontend) run in CI, not locally — none is
silently skipped.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Every JNI export wraps its body in `jni_catch_unwind_*` so a Rust panic
cannot unwind across the `extern "system"` boundary (which is UB and
aborts the JVM). `finalizeScannerPairing` was the lone exception: it
called `env.get_string(..)` and `rt.block_on(..)` directly, so a panic in
the orchestrator/runtime would cross the boundary.

Wrap the body in `jni_catch_unwind_jboolean` (returns JNI_FALSE / 0 on
panic), matching the other exports.

NOTE: `jni` is `#[cfg(all(target_os = "android", feature = "jni"))]`, not
built by the host toolchain; rustfmt will normalize the body indentation
on an Android build. Verify there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dsmfa10 dsmfa10 force-pushed the fix/sdk-jni-finalize-pairing-panic-guard branch from 623c00c to c1aeaad Compare June 3, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants