Fix: Callback safety - #124
Open
redaranj wants to merge 4 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/sign-from-context #124 +/- ##
=========================================================
Coverage 72.21% 72.21%
Complexity 38 38
=========================================================
Files 62 62
Lines 2350 2350
Branches 286 286
=========================================================
Hits 1697 1697
Misses 494 494
Partials 159 159 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Reference-count JavaSignerContext and JavaContextCallback under registry mutexes: each in-flight callback invocation acquires the context on entry and releases it on exit, and freeing the owning signer or context marks it inactive and drops the owner's reference, deferring the free and DeleteGlobalRef until the last invocation completes. The previously unsynchronized isActive flag is now read and written only under the mutex. Adds a shared test that closes a context while a derived builder's progress callback is in flight.
A Java exception raised inside a stream, signer, or HTTP-resolver callback was previously thrown or described-and-cleared before returning -1 into Rust. The thrown variants left the exception pending while the FFI kept making JNI calls, which is undefined behavior, and the cleared variants swallowed the app's real exception behind a generic RuntimeException. Callbacks now clear and stash the throwable in a thread-local slot, and the outer JNI boundary rethrows it: throw_c2pa_exception prefers the stashed throwable, the sign functions rethrow it before their sentinel return, and callback-driving entry points drop any stale stash on entry. Oversize-buffer conditions report through c2pa_error_set_last instead of throwing. Progress-observer exceptions are still logged and dropped by design. Adds a shared test asserting a stream's IOException surfaces to the caller as itself.
The thread-local stash was cleared at entry by callback-running entry points but only rethrown by the sign paths, so a stream or signer exception raised during toArchive, addResource, resourceToStream and the other result-code entry points was replaced by a generic C2PAError and left in the stash, where the next unrelated failure on the same thread could resurface it from an entry point that never ran a callback. Add finish_stashed_exception, which rethrows the stash when the FFI call failed and drops it when it succeeded, and call it on every exit after the FFI call in all seventeen entry points that run callbacks. Stop throw_c2pa_exception from consulting the stash implicitly so entry points without callbacks can never surface a stale one. Add a shared test that breaks the write callback in toArchive, asserts the IOException surfaces, then fails an unrelated call on the same thread and asserts the IOException does not come back. Co-Authored-By: Claude Fable 5 <[email protected]>
The registry lookup that guards late callbacks compared the user_data handed back by the core against live struct pointers. Once a context was freed, malloc could hand the same address to the next one, so a late callback for a closed signer or context could match the new entry and invoke the wrong app callback. Give each signer context and context callback a never-reused id from a mutex-guarded counter, pass that id to the core as user_data, and have the trampolines resolve it through the registry instead of casting it to a pointer. Registration, unregistration and the Kotlin-facing handles still operate on struct pointers held only on the JNI side. Co-Authored-By: Claude Fable 5 <[email protected]>
redaranj
force-pushed
the
fix/callback-safety
branch
from
August 21, 2026 06:37
2b61646 to
b3e7f39
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes in this pull request
Fixes two races in the JNI callback layer: callback contexts are now registry-tracked and reference-counted, so closing a signer or context during an in-flight operation can no longer touch freed memory. Java exceptions thrown inside stream and signer callbacks are stashed and rethrown at the JNI boundary instead of being left pending inside Rust or swallowed behind a generic RuntimeException.
Types of changes
Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment