proposal(profiling-ffi): catch_unwind via panic bit on ProfileStatus#1970
proposal(profiling-ffi): catch_unwind via panic bit on ProfileStatus#1970r1viollet wants to merge 1 commit into
Conversation
Exploration PR — one of three sibling proposals for adding catch_unwind at the profiling FFI boundary. Adds a panic-bit flag on ProfileStatus so caught panics surface in-band on the existing return type, with no ABI break and zero overhead on the OK path. - New IS_PANIC_MASK (0x2) on ProfileStatus.flags. - New wrap_with_profile_status! macro: catch_unwind around the body, sets panic bit on caught payloads. - Nested catch_unwind in from_panic for OOM-safe message formatting, mirroring libdd-common-ffi::handle_panic_error. - One example migration: ddog_prof_ProfilesDictionary_insert_function. - New ddog_prof_Status_is_panic accessor for C callers. - Fix: From<ProfileStatus> for Result now masks IS_ALLOCATED_MASK rather than checking exact equality — prevents heap leak when other bits coexist. See docs/proposals/profiling-ffi-catch-unwind-bit-flag.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results📦
|
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1970 +/- ##
==========================================
- Coverage 72.63% 72.61% -0.02%
==========================================
Files 448 448
Lines 73582 73650 +68
==========================================
+ Hits 53444 53482 +38
- Misses 20138 20168 +30
🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 0b94194 | Docs | Datadog PR Page | Give us feedback! |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
Summary
Exploration PR — one of three sibling proposals for adding
catch_unwindat the profiling FFI boundary. Not for merge as-is; intended to make the API shape concrete enough to choose between.This variant encodes "this status came from a caught panic" as a bit on
ProfileStatus.flags. The OK representation ({0, null}) is unchanged, so existing C consumers keep working and just see another error.IS_PANIC_MASK = 0x2flag onProfileStatus.flags.wrap_with_profile_status!macro:catch_unwindaround the body, ORs the panic bit on caught payloads. Mirrors the nested-catch_unwindOOM fallback inlibdd-common-ffi::handle_panic_error.ddog_prof_Status_is_panicaccessor for C callers to differentiate panic from normal error.ddog_prof_ProfilesDictionary_insert_function. OtherProfileStatus-returning functions are untouched in this PR.From<ProfileStatus> for Resultnow masksIS_ALLOCATED_MASKrather than==, otherwise coexisting bits would silently leak the heap message.Full design doc:
docs/proposals/profiling-ffi-catch-unwind-bit-flag.md.Sibling proposals
r1viollet/profiling-ffi-panic-callback— global panic callback.r1viollet/profiling-ffi-panic-bit-and-callback— both, combined.Pick one (or none) of the three to take forward to a real RFC.
Test plan
cargo check -p libdd-profiling-ffi(done locally)cargo test -p libdd-profiling-fficovering the newis_panicaccessor andfrom_panicformattingwrap_with_profile_status!→ status with panic bit, non-null err, no leak afterddog_prof_Status_drop{0, null}for OK, allocated-vs-static still round-trips)🤖 Generated with Claude Code