From 496b71a959794898f22b121500e9c64f3c3d117b Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 19:51:47 -0700 Subject: [PATCH 1/9] fix: Lifetimes --- .claude/settings.json | 10 ++++ include/c2pa.hpp | 72 +++++++++++++++++++----- src/c2pa_context.cpp | 72 +++++++++++++++--------- tests/context.test.cpp | 123 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 238 insertions(+), 39 deletions(-) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..637d370 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,10 @@ +{ + "permissions": { + "allow": [ + "mcp__plugin_context-mode_context-mode__ctx_execute", + "mcp__plugin_context-mode_context-mode__ctx_execute_file", + "mcp__plugin_context-mode_context-mode__ctx_search", + "Bash(cmake -S . -B build/debug -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON)" + ] + } +} diff --git a/include/c2pa.hpp b/include/c2pa.hpp index 46767a9..0a0d5a5 100644 --- a/include/c2pa.hpp +++ b/include/c2pa.hpp @@ -115,9 +115,19 @@ namespace c2pa /// @brief Interface for types that can provide C2PA context functionality. /// @details This interface can be implemented by external libraries to provide /// custom context implementations (e.g. AdobeContext wrappers). - /// Reader and Builder take the context by reference and use it only at - /// construction; the underlying implementation copies context state - /// into the reader/builder, so the context does not need to outlive them. + /// Reader and Builder accept a shared_ptr and extend + /// the provider's lifetime for as long as the Reader/Builder exists. + /// + /// @par Progress callback lifetime (post-0322d67) + /// If the native C2paContext* held by the provider was configured with a progress + /// callback, the provider MUST keep the heap-owned ProgressCallbackFunc alive at + /// least as long as the native context. The native side stores only a raw pointer + /// into that heap block and will call it until the context is freed. Destroy + /// order inside the provider: free the native C2paContext* first (stops further + /// callback invocations), then release the callback storage. The built-in + /// Context class enforces this via member declaration order; external providers + /// that adopt a native context built via ContextBuilder::release() must do the + /// same. /// /// @par Move semantics /// Move construction and move assignment are defaulted. After move, the moved-from @@ -404,16 +414,41 @@ namespace c2pa /// @note This consumes the builder. After calling this, is_valid() returns false. [[nodiscard]] Context create_context(); - /// @brief Release ownership of the underlying C2paContextBuilder pointer. - /// After this call, the ContextBuilder no longer owns the pointer - /// and is_valid() returns false. The caller is responsible for managing - /// the lifetime of the returned pointer. - /// @return Pointer to the C2paContextBuilder object, or nullptr if moved from. - C2paContextBuilder* release() noexcept; + /// @brief Result of ContextBuilder::release(): the raw native builder handle + /// paired with the heap-owned progress callback (if any). + /// @details The caller takes joint ownership. If `callback_owner` is non-null, + /// the native side stores a raw pointer into it and will invoke the + /// callback until the native context built from `builder` is freed. + /// Therefore `callback_owner` must remain alive at least until the + /// native context returned by c2pa_context_builder_build(builder) is + /// freed via c2pa_free(). Release order: native context first, then + /// `callback_owner` goes out of scope. + struct ReleasedBuilder { + C2paContextBuilder* builder; + std::unique_ptr callback_owner; + }; + + /// @brief Release ownership of the underlying C2paContextBuilder handle and + /// its progress-callback heap block (if any) to the caller. + /// @details After this call is_valid() returns false. The previous + /// C2paContextBuilder* overload was unsound when a progress callback + /// was pending: the C++ builder destructor would free the heap + /// ProgressCallbackFunc while the native side still held its pointer, + /// yielding a use-after-free on the first progress tick. + /// Callers must keep ReleasedBuilder::callback_owner alive for as + /// long as the native context built from ReleasedBuilder::builder + /// exists. + /// @return ReleasedBuilder holding both the native builder and (optionally) + /// the owning unique_ptr for the progress callback. Fields are null + /// when moved-from. + [[nodiscard]] ReleasedBuilder release() noexcept; private: - C2paContextBuilder* context_builder; + // pending_callback_ is declared before context_builder + // so destruction (reverse of declaration) frees context_builder first, while + // the callback heap block is still live, then releases pending_callback_. std::unique_ptr pending_callback_; + C2paContextBuilder* context_builder; }; // Direct construction @@ -464,6 +499,13 @@ namespace c2pa /// @throws C2paException if ctx is nullptr. explicit Context(C2paContext* ctx); + /// @brief "Adopt" a native context together with its heap-owned progress callback. + /// @param ctx Native context pointer. Context takes ownership. + /// @param callback Heap-owned progress callback whose raw pointer the native + /// context may hold. May be null. + Context(C2paContext* ctx, + std::unique_ptr callback) noexcept; + /// @brief Request cancellation of any in-progress operation on this context. /// /// @details Safe to call from another thread while this Context remains valid @@ -476,11 +518,15 @@ namespace c2pa void cancel() noexcept; private: + // Member order matters: `context` is declared before `callback_owner_` so + // destruction (reverse of declaration) frees the heap callback block after + // the native context has been freed. C2paContext* context; - /// Heap-owned ProgressCallbackFunc; non-null only when set via - /// ContextBuilder::with_progress_callback(). Deleted in the destructor. - void* callback_owner_ = nullptr; + /// Heap-owned ProgressCallbackFunc, non-null only when the Context was built + /// with a progress callback via ContextBuilder::with_progress_callback(). + /// Destroyed after `context`. + std::unique_ptr callback_owner_; }; /// @brief Get the version of the C2PA library. diff --git a/src/c2pa_context.cpp b/src/c2pa_context.cpp index 616bae9..c025cf2 100644 --- a/src/c2pa_context.cpp +++ b/src/c2pa_context.cpp @@ -40,22 +40,32 @@ namespace c2pa Context::Context(Context&& other) noexcept : context(std::exchange(other.context, nullptr)), - callback_owner_(std::exchange(other.callback_owner_, nullptr)) { + callback_owner_(std::move(other.callback_owner_)) { } Context& Context::operator=(Context&& other) noexcept { if (this != &other) { - c2pa_free(context); - delete static_cast(callback_owner_); + // Free the native context first so it stops invoking the callback, + // then release the heap block the callback lived in (move). + if (context) { + c2pa_free(context); + } context = std::exchange(other.context, nullptr); - callback_owner_ = std::exchange(other.callback_owner_, nullptr); + callback_owner_ = std::move(other.callback_owner_); } return *this; } Context::~Context() noexcept { - c2pa_free(context); - delete static_cast(callback_owner_); + // Free native context first, then let callback_owner_ destruct. + if (context) { + c2pa_free(context); + } + } + + Context::Context(C2paContext* ctx, + std::unique_ptr callback) noexcept + : context(ctx), callback_owner_(std::move(callback)) { } void Context::cancel() noexcept { @@ -91,13 +101,18 @@ namespace c2pa } Context::ContextBuilder::ContextBuilder(ContextBuilder&& other) noexcept - : context_builder(std::exchange(other.context_builder, nullptr)), - pending_callback_(std::move(other.pending_callback_)) { + : pending_callback_(std::move(other.pending_callback_)), + context_builder(std::exchange(other.context_builder, nullptr)) { } Context::ContextBuilder& Context::ContextBuilder::operator=(ContextBuilder&& other) noexcept { if (this != &other) { - c2pa_free(context_builder); + // Free the native builder before moving in the new pending_callback_ so + // the old native builder cannot touch its (about-to-be-replaced) callback + // block after we've dropped the old one. + if (context_builder) { + c2pa_free(context_builder); + } context_builder = std::exchange(other.context_builder, nullptr); pending_callback_ = std::move(other.pending_callback_); } @@ -105,7 +120,12 @@ namespace c2pa } Context::ContextBuilder::~ContextBuilder() noexcept { - c2pa_free(context_builder); + // Member destruction order is reverse of declaration: + // context_builder freed first (stops FFI from calling the callback), + // then pending_callback_ is released. + if (context_builder) { + c2pa_free(context_builder); + } } bool Context::ContextBuilder::is_valid() const noexcept { @@ -225,21 +245,27 @@ namespace c2pa if (!is_valid()) { throw C2paException("ContextBuilder is invalid (moved from)"); } - // Heap-allocate the std::function so we can pass a stable pointer to the C API. - // The resulting Context takes ownership of this allocation. - pending_callback_ = std::make_unique(std::move(callback)); + // Two-stage swap: create the new heap block, register it with the FFI, and + // only after drop any previous pending_callback_. + auto fresh = std::make_unique(std::move(callback)); if (c2pa_context_builder_set_progress_callback( context_builder, - pending_callback_.get(), + fresh.get(), progress_callback_trampoline) != 0) { - pending_callback_.reset(); throw C2paException(); } + // FFI now references `fresh`. Safe to drop any previous block. + pending_callback_ = std::move(fresh); return *this; } - C2paContextBuilder* Context::ContextBuilder::release() noexcept { - return std::exchange(context_builder, nullptr); + Context::ContextBuilder::ReleasedBuilder Context::ContextBuilder::release() noexcept { + // Transfer both the native builder handle and the callback heap block to + // the caller atomically. + return ReleasedBuilder{ + std::exchange(context_builder, nullptr), + std::move(pending_callback_), + }; } Context Context::ContextBuilder::create_context() { @@ -247,19 +273,13 @@ namespace c2pa throw C2paException("ContextBuilder is invalid (moved from)"); } - // The C API consumes the context builder on build + // The C API consumes the context builder on build (success or failure). C2paContext* ctx = c2pa_context_builder_build(context_builder); + context_builder = nullptr; if (!ctx) { throw C2paException("Failed to build context"); } - // Builder is consumed by the C API - context_builder = nullptr; - - Context result(ctx); - // Transfer progress callback heap ownership to the Context so it is freed - // when the Context is destroyed (the C side holds a raw pointer to it). - result.callback_owner_ = pending_callback_.release(); - return result; + return Context(ctx, std::move(pending_callback_)); } } // namespace c2pa diff --git a/tests/context.test.cpp b/tests/context.test.cpp index 8a943bf..0fabe71 100644 --- a/tests/context.test.cpp +++ b/tests/context.test.cpp @@ -758,3 +758,126 @@ TEST_F(ContextTest, SharedPtrContext_FromAsset) { EXPECT_TRUE(reader.has_value()); EXPECT_EQ(ctx.use_count(), 2); // ctx + reader } + +// ContextBuilder::release() must carry the progress-callback owner +// alongside the raw native builder handle. +TEST_F(ContextTest, ContextBuilderReleaseCarriesCallbackOwner) { + std::atomic call_count{0}; + + // Minimal external IContextProvider that adopts the ReleasedBuilder + // pieces and preserves the required destruction order (native context + // first, then callback storage). + class AdoptingProvider : public c2pa::IContextProvider { + public: + AdoptingProvider(c2pa::Context::ContextBuilder::ReleasedBuilder rb) { + if (!rb.builder) { + throw c2pa::C2paException("AdoptingProvider: null builder"); + } + ctx_ = c2pa_context_builder_build(rb.builder); + if (!ctx_) { + throw c2pa::C2paException("AdoptingProvider: build failed"); + } + callback_owner_ = std::move(rb.callback_owner); + } + ~AdoptingProvider() noexcept override { + if (ctx_) { + c2pa_free(ctx_); + } + // callback_owner_ destructs after this body -- correct order. + } + AdoptingProvider(const AdoptingProvider&) = delete; + AdoptingProvider& operator=(const AdoptingProvider&) = delete; + + C2paContext* c_context() const noexcept override { return ctx_; } + bool is_valid() const noexcept override { return ctx_ != nullptr; } + + private: + C2paContext* ctx_ = nullptr; + std::unique_ptr callback_owner_; + }; + + auto rb = c2pa::Context::ContextBuilder() + .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++call_count; + return true; + }) + .release(); + ASSERT_NE(rb.builder, nullptr); + ASSERT_NE(rb.callback_owner, nullptr); + + auto provider = std::make_shared(std::move(rb)); + ASSERT_TRUE(provider->is_valid()); + + EXPECT_NO_THROW(sign_with_progress_context(provider, get_temp_path("release_adopting.jpg"))); + EXPECT_GT(call_count.load(), 0); +} + +// A ContextBuilder that has a progress callback set but is never +// turned into a Context must destruct cleanly. +TEST_F(ContextTest, ContextBuilderDroppedAfterCallbackNoUAF) { + std::atomic call_count{0}; + { + c2pa::Context::ContextBuilder b; + b.with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++call_count; + return true; + }); + // Intentionally no create_context(); let b destruct here. + } + // Nothing crashed, nothing leaked. ASan is the real judge. + SUCCEED(); +} + +// Ccreate_context() must transfer callback ownership to the new +// Context atomically. +TEST_F(ContextTest, ContextBuilderCreateContextAtomicHandoff) { + std::atomic call_count{0}; + + // Builder goes out of scope the moment create_context() returns. + // If the handoff were not atomic, the callback heap block would be + // in a race with builder destruction. + auto context = std::make_shared(c2pa::Context::ContextBuilder() + .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++call_count; + return true; + }) + .create_context()); + + ASSERT_TRUE(context->is_valid()); + EXPECT_NO_THROW(sign_with_progress_context(context, get_temp_path("atomic_handoff.jpg"))); + EXPECT_GT(call_count.load(), 0); +} + +// Context move-assignment must free the destination's native context +// before releasing its old callback storage, and must leave the survivor +// with a valid callback bound to its (new) native context. +TEST_F(ContextTest, ContextMoveAssignWithCallbackPreservesInvocation) { + std::atomic lhs{0}; + std::atomic rhs{0}; + + c2pa::Context a = c2pa::Context::ContextBuilder() + .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++lhs; + return true; + }) + .create_context(); + + c2pa::Context b = c2pa::Context::ContextBuilder() + .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { + ++rhs; + return true; + }) + .create_context(); + + a = std::move(b); + EXPECT_FALSE(b.is_valid()); + ASSERT_TRUE(a.is_valid()); + + auto sp = std::make_shared(std::move(a)); + EXPECT_NO_THROW(sign_with_progress_context(sp, get_temp_path("move_assign_survivor.jpg"))); + + // a's callback block was freed when `a = std::move(b)` ran; it must + // not fire. Only b's (now owned by the survivor) may fire. + EXPECT_EQ(lhs.load(), 0); + EXPECT_GT(rhs.load(), 0); +} From 284196362b03c4f94c2f9bb99c9ca4f629d6fc15 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 19:54:40 -0700 Subject: [PATCH 2/9] fix: Lifetimes --- .claude/settings.json | 10 ---------- docs/context-settings.md | 1 + tests/context.test.cpp | 2 +- 3 files changed, 2 insertions(+), 11 deletions(-) delete mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json deleted file mode 100644 index 637d370..0000000 --- a/.claude/settings.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "permissions": { - "allow": [ - "mcp__plugin_context-mode_context-mode__ctx_execute", - "mcp__plugin_context-mode_context-mode__ctx_execute_file", - "mcp__plugin_context-mode_context-mode__ctx_search", - "Bash(cmake -S . -B build/debug -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON)" - ] - } -} diff --git a/docs/context-settings.md b/docs/context-settings.md index bf6e767..5a48690 100644 --- a/docs/context-settings.md +++ b/docs/context-settings.md @@ -165,6 +165,7 @@ auto context = c2pa::Context::ContextBuilder() | `with_signer(signer)` | Store a `Signer` in the context (consumed; used by `Builder::sign` with no explicit signer) | | `with_progress_callback(callback)` | Register a progress/cancel callback (see [Progress callbacks and cancellation](#progress-callbacks-and-cancellation)) | | `create_context()` | Build and return the `Context` (consumes the builder) | +| `release()` | Release the raw `C2paContextBuilder*` together with its progress-callback heap owner (see [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider)) | ## Progress callbacks and cancellation diff --git a/tests/context.test.cpp b/tests/context.test.cpp index 0fabe71..c0723b4 100644 --- a/tests/context.test.cpp +++ b/tests/context.test.cpp @@ -850,7 +850,7 @@ TEST_F(ContextTest, ContextBuilderCreateContextAtomicHandoff) { // Context move-assignment must free the destination's native context // before releasing its old callback storage, and must leave the survivor -// with a valid callback bound to its (new) native context. +// with a valid callback bound to its new context. TEST_F(ContextTest, ContextMoveAssignWithCallbackPreservesInvocation) { std::atomic lhs{0}; std::atomic rhs{0}; From 59b88f13aaa3e9f4ed8218941e46e557a3d98226 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 19:56:31 -0700 Subject: [PATCH 3/9] fix: Lifetimes --- docs/context-settings.md | 53 ++++++++++++++++++++++++++++++++++++++++ tests/context.test.cpp | 13 +++------- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/docs/context-settings.md b/docs/context-settings.md index 5a48690..8cb1450 100644 --- a/docs/context-settings.md +++ b/docs/context-settings.md @@ -206,6 +206,9 @@ bool callback(c2pa::ProgressPhase phase, uint32_t step, uint32_t total); **Do not throw** from the progress callback. Exceptions cannot cross the C/Rust boundary safely; if your callback throws, the wrapper catches it and the operation is aborted as a cancellation (you do not get your exception back at the call site). Use `return false`, `context.cancel()`, or application-side state instead. +> [!IMPORTANT] +> **Callback lifetime.** The callback you pass to `with_progress_callback` is copied onto the heap and its address is handed to the underlying library. The built-in `Context` owns that heap block and frees it only after the native context is freed, so the call order (native context first, then the callback storage) is always correct when you use `create_context()` plus `shared_ptr`. If you instead transfer the native handle out of the builder via `release()`, you take over both halves of that contract; see [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider). + ### Cancelling from another thread You may call `Context::cancel()` from another thread while the same `Context` remains valid and is not being destroyed or moved concurrently with that call. The SDK returns a `C2paException` with an `OperationCancelled` error at the next progress checkpoint: @@ -979,6 +982,56 @@ The `shared_ptr` overloads accept any `shared_ptr`, so custom External libraries can implement `IContextProvider` to supply their own context objects. The interface requires a valid `C2paContext*` pointer and an `is_valid()` check. Wrap your implementation in a `shared_ptr` when passing to `Reader` or `Builder`. +> [!IMPORTANT] +> **Progress-callback lifetime in custom providers.** If the native `C2paContext*` was built with a progress callback, the underlying library stores a raw pointer into a heap-owned `ProgressCallbackFunc`. Your provider MUST keep that callback alive for at least as long as the native context. When destroying the provider, free the `C2paContext*` first (so the library stops invoking the callback) and release the callback storage after. The built-in `Context` enforces this via member declaration order; custom providers must do the same. + +#### Releasing to a custom IContextProvider + +`Context::ContextBuilder::release()` hands off both the native `C2paContextBuilder*` and the heap-owned progress callback (if any) to the caller. Use it when you need to build the native context yourself or adopt it into a custom `IContextProvider`. + +```cpp +class MyContextProvider : public c2pa::IContextProvider { +public: + explicit MyContextProvider(c2pa::Context::ContextBuilder::ReleasedBuilder rb) { + // The builder and the callback owner come as a pair; take both. + ctx_ = c2pa_context_builder_build(rb.builder); + if (!ctx_) { + throw c2pa::C2paException("Failed to build context"); + } + callback_owner_ = std::move(rb.callback_owner); + } + + ~MyContextProvider() noexcept override { + // Order matters: free the native context FIRST (stops further callback + // invocations), then let callback_owner_ destruct. + if (ctx_) { + c2pa_free(ctx_); + } + } + + C2paContext* c_context() const noexcept override { return ctx_; } + bool is_valid() const noexcept override { return ctx_ != nullptr; } + +private: + C2paContext* ctx_ = nullptr; + std::unique_ptr callback_owner_; +}; + +auto rb = c2pa::Context::ContextBuilder() + .with_progress_callback([](c2pa::ProgressPhase, uint32_t, uint32_t) { return true; }) + .release(); + +auto provider = std::make_shared(std::move(rb)); +c2pa::Reader reader(provider, "image.jpg"); +``` + +`ReleasedBuilder` exposes two fields: + +- `builder` — the raw `C2paContextBuilder*`. Pass to `c2pa_context_builder_build` exactly once. +- `callback_owner` — `std::unique_ptr`; `nullptr` when no progress callback was set. Whenever it is non-null you must keep it alive at least as long as the native context built from `builder`, otherwise the library will dereference freed memory on the next progress tick. + +If you only need the built-in `Context`, prefer `create_context()` — it performs the same handoff internally and removes any chance of splitting the pair. + ### Builder::sign overloads `Builder::sign` has two kinds of overloads: those that take an explicit `Signer` argument, and those that use the signer stored in the `Builder`'s `Context`. diff --git a/tests/context.test.cpp b/tests/context.test.cpp index c0723b4..6f0d226 100644 --- a/tests/context.test.cpp +++ b/tests/context.test.cpp @@ -765,8 +765,7 @@ TEST_F(ContextTest, ContextBuilderReleaseCarriesCallbackOwner) { std::atomic call_count{0}; // Minimal external IContextProvider that adopts the ReleasedBuilder - // pieces and preserves the required destruction order (native context - // first, then callback storage). + // pieces and preserves the required destruction order. class AdoptingProvider : public c2pa::IContextProvider { public: AdoptingProvider(c2pa::Context::ContextBuilder::ReleasedBuilder rb) { @@ -783,7 +782,6 @@ TEST_F(ContextTest, ContextBuilderReleaseCarriesCallbackOwner) { if (ctx_) { c2pa_free(ctx_); } - // callback_owner_ destructs after this body -- correct order. } AdoptingProvider(const AdoptingProvider&) = delete; AdoptingProvider& operator=(const AdoptingProvider&) = delete; @@ -822,9 +820,9 @@ TEST_F(ContextTest, ContextBuilderDroppedAfterCallbackNoUAF) { ++call_count; return true; }); - // Intentionally no create_context(); let b destruct here. + // Intentionally no create_context(), let b destruct here. } - // Nothing crashed, nothing leaked. ASan is the real judge. + SUCCEED(); } @@ -833,9 +831,6 @@ TEST_F(ContextTest, ContextBuilderDroppedAfterCallbackNoUAF) { TEST_F(ContextTest, ContextBuilderCreateContextAtomicHandoff) { std::atomic call_count{0}; - // Builder goes out of scope the moment create_context() returns. - // If the handoff were not atomic, the callback heap block would be - // in a race with builder destruction. auto context = std::make_shared(c2pa::Context::ContextBuilder() .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) { ++call_count; @@ -876,8 +871,6 @@ TEST_F(ContextTest, ContextMoveAssignWithCallbackPreservesInvocation) { auto sp = std::make_shared(std::move(a)); EXPECT_NO_THROW(sign_with_progress_context(sp, get_temp_path("move_assign_survivor.jpg"))); - // a's callback block was freed when `a = std::move(b)` ran; it must - // not fire. Only b's (now owned by the survivor) may fire. EXPECT_EQ(lhs.load(), 0); EXPECT_GT(rhs.load(), 0); } From e53f1b4501ac34108814b7873381c66226224fc6 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:10:44 -0700 Subject: [PATCH 4/9] fix: Docs --- .claude/settings.json | 7 +++++++ docs/context-settings.md | 12 ++++++------ src/c2pa_context.cpp | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..8e48137 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,7 @@ +{ + "permissions": { + "allow": [ + "Bash(rtk git *)" + ] + } +} diff --git a/docs/context-settings.md b/docs/context-settings.md index 8cb1450..b6eec39 100644 --- a/docs/context-settings.md +++ b/docs/context-settings.md @@ -207,7 +207,7 @@ bool callback(c2pa::ProgressPhase phase, uint32_t step, uint32_t total); **Do not throw** from the progress callback. Exceptions cannot cross the C/Rust boundary safely; if your callback throws, the wrapper catches it and the operation is aborted as a cancellation (you do not get your exception back at the call site). Use `return false`, `context.cancel()`, or application-side state instead. > [!IMPORTANT] -> **Callback lifetime.** The callback you pass to `with_progress_callback` is copied onto the heap and its address is handed to the underlying library. The built-in `Context` owns that heap block and frees it only after the native context is freed, so the call order (native context first, then the callback storage) is always correct when you use `create_context()` plus `shared_ptr`. If you instead transfer the native handle out of the builder via `release()`, you take over both halves of that contract; see [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider). +> **Callback lifetime.** A callback passed to `with_progress_callback` is copied onto the heap and its address is handed to the underlying library. The built-in `Context` owns that heap block and frees it only after the native context is freed. The free order is correct by construction when the callback flows through `create_context()` into a `shared_ptr`. When the native handle is transferred out of the builder via `release()`, the same contract must be maintained by the caller. See [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider). ### Cancelling from another thread @@ -983,11 +983,11 @@ The `shared_ptr` overloads accept any `shared_ptr`, so custom External libraries can implement `IContextProvider` to supply their own context objects. The interface requires a valid `C2paContext*` pointer and an `is_valid()` check. Wrap your implementation in a `shared_ptr` when passing to `Reader` or `Builder`. > [!IMPORTANT] -> **Progress-callback lifetime in custom providers.** If the native `C2paContext*` was built with a progress callback, the underlying library stores a raw pointer into a heap-owned `ProgressCallbackFunc`. Your provider MUST keep that callback alive for at least as long as the native context. When destroying the provider, free the `C2paContext*` first (so the library stops invoking the callback) and release the callback storage after. The built-in `Context` enforces this via member declaration order; custom providers must do the same. +> **Progress-callback lifetime in custom providers.** If the native `C2paContext*` was built with a progress callback, the underlying library stores a raw pointer into a heap-owned `ProgressCallbackFunc`. A provider implementation must keep that callback alive for at least as long as the native context. When destroying the provider, the `C2paContext*` must be freed first and the callback storage released after. #### Releasing to a custom IContextProvider -`Context::ContextBuilder::release()` hands off both the native `C2paContextBuilder*` and the heap-owned progress callback (if any) to the caller. Use it when you need to build the native context yourself or adopt it into a custom `IContextProvider`. +`Context::ContextBuilder::release()` hands off both the native `C2paContextBuilder*` and the heap-owned progress callback (if any) to the caller. It is the entry point when application code needs to build the native context directly or adopt it into a custom `IContextProvider` implementation. ```cpp class MyContextProvider : public c2pa::IContextProvider { @@ -1027,10 +1027,10 @@ c2pa::Reader reader(provider, "image.jpg"); `ReleasedBuilder` exposes two fields: -- `builder` — the raw `C2paContextBuilder*`. Pass to `c2pa_context_builder_build` exactly once. -- `callback_owner` — `std::unique_ptr`; `nullptr` when no progress callback was set. Whenever it is non-null you must keep it alive at least as long as the native context built from `builder`, otherwise the library will dereference freed memory on the next progress tick. +- `builder`: the raw `C2paContextBuilder*`. It must be passed to `c2pa_context_builder_build` exactly once. +- `callback_owner`: `std::unique_ptr`; `nullptr` when no progress callback was set. When non-null, it must remain alive at least as long as the native context built from `builder`. If it is destroyed earlier, the library will dereference freed memory on the next progress tick. -If you only need the built-in `Context`, prefer `create_context()` — it performs the same handoff internally and removes any chance of splitting the pair. +When the built-in `Context` is sufficient, `create_context()` is preferred. It performs the same handoff internally and keeps the native handle and callback owner bound together. ### Builder::sign overloads diff --git a/src/c2pa_context.cpp b/src/c2pa_context.cpp index c025cf2..bcecb7c 100644 --- a/src/c2pa_context.cpp +++ b/src/c2pa_context.cpp @@ -245,6 +245,7 @@ namespace c2pa if (!is_valid()) { throw C2paException("ContextBuilder is invalid (moved from)"); } + // Two-stage swap: create the new heap block, register it with the FFI, and // only after drop any previous pending_callback_. auto fresh = std::make_unique(std::move(callback)); @@ -254,7 +255,6 @@ namespace c2pa progress_callback_trampoline) != 0) { throw C2paException(); } - // FFI now references `fresh`. Safe to drop any previous block. pending_callback_ = std::move(fresh); return *this; } From be01baaa8f43fd48bfa031da509955750096c3f5 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:12:05 -0700 Subject: [PATCH 5/9] fix: Docs --- .claude/settings.json | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json deleted file mode 100644 index 8e48137..0000000 --- a/.claude/settings.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "permissions": { - "allow": [ - "Bash(rtk git *)" - ] - } -} From 36366f618b21b3cda9c1852b05cb011b8d265483 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:12:56 -0700 Subject: [PATCH 6/9] fix: Docs --- docs/context-settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/context-settings.md b/docs/context-settings.md index b6eec39..e91bb86 100644 --- a/docs/context-settings.md +++ b/docs/context-settings.md @@ -207,7 +207,7 @@ bool callback(c2pa::ProgressPhase phase, uint32_t step, uint32_t total); **Do not throw** from the progress callback. Exceptions cannot cross the C/Rust boundary safely; if your callback throws, the wrapper catches it and the operation is aborted as a cancellation (you do not get your exception back at the call site). Use `return false`, `context.cancel()`, or application-side state instead. > [!IMPORTANT] -> **Callback lifetime.** A callback passed to `with_progress_callback` is copied onto the heap and its address is handed to the underlying library. The built-in `Context` owns that heap block and frees it only after the native context is freed. The free order is correct by construction when the callback flows through `create_context()` into a `shared_ptr`. When the native handle is transferred out of the builder via `release()`, the same contract must be maintained by the caller. See [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider). +> A callback passed to `with_progress_callback` is copied onto the heap and its address is handed to the underlying library. The built-in `Context` owns that heap block and frees it only after the native context is freed. The free order is correct by construction when the callback flows through `create_context()` into a `shared_ptr`. When the native handle is transferred out of the builder via `release()`, the same contract must be maintained by the caller. See [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider). ### Cancelling from another thread From cac66334750b3070c1b4e57920f0ac7bb626ef30 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:14:03 -0700 Subject: [PATCH 7/9] fix: Docs 2 --- docs/context-settings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/context-settings.md b/docs/context-settings.md index e91bb86..d87f31e 100644 --- a/docs/context-settings.md +++ b/docs/context-settings.md @@ -165,7 +165,7 @@ auto context = c2pa::Context::ContextBuilder() | `with_signer(signer)` | Store a `Signer` in the context (consumed; used by `Builder::sign` with no explicit signer) | | `with_progress_callback(callback)` | Register a progress/cancel callback (see [Progress callbacks and cancellation](#progress-callbacks-and-cancellation)) | | `create_context()` | Build and return the `Context` (consumes the builder) | -| `release()` | Release the raw `C2paContextBuilder*` together with its progress-callback heap owner (see [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider)) | +| `release()` | Release the raw `C2paContextBuilder*` together with its progress-callback heap owner (see also [Releasing to a custom IContextProvider](#releasing-to-a-custom-icontextprovider)) | ## Progress callbacks and cancellation From eaf1ab508124a0a06b1829be3a3e8aefbcfab6c8 Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Fri, 17 Apr 2026 07:08:01 -0700 Subject: [PATCH 8/9] Update context-settings.md --- docs/context-settings.md | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/docs/context-settings.md b/docs/context-settings.md index d87f31e..a10fb2c 100644 --- a/docs/context-settings.md +++ b/docs/context-settings.md @@ -989,42 +989,6 @@ External libraries can implement `IContextProvider` to supply their own context `Context::ContextBuilder::release()` hands off both the native `C2paContextBuilder*` and the heap-owned progress callback (if any) to the caller. It is the entry point when application code needs to build the native context directly or adopt it into a custom `IContextProvider` implementation. -```cpp -class MyContextProvider : public c2pa::IContextProvider { -public: - explicit MyContextProvider(c2pa::Context::ContextBuilder::ReleasedBuilder rb) { - // The builder and the callback owner come as a pair; take both. - ctx_ = c2pa_context_builder_build(rb.builder); - if (!ctx_) { - throw c2pa::C2paException("Failed to build context"); - } - callback_owner_ = std::move(rb.callback_owner); - } - - ~MyContextProvider() noexcept override { - // Order matters: free the native context FIRST (stops further callback - // invocations), then let callback_owner_ destruct. - if (ctx_) { - c2pa_free(ctx_); - } - } - - C2paContext* c_context() const noexcept override { return ctx_; } - bool is_valid() const noexcept override { return ctx_ != nullptr; } - -private: - C2paContext* ctx_ = nullptr; - std::unique_ptr callback_owner_; -}; - -auto rb = c2pa::Context::ContextBuilder() - .with_progress_callback([](c2pa::ProgressPhase, uint32_t, uint32_t) { return true; }) - .release(); - -auto provider = std::make_shared(std::move(rb)); -c2pa::Reader reader(provider, "image.jpg"); -``` - `ReleasedBuilder` exposes two fields: - `builder`: the raw `C2paContextBuilder*`. It must be passed to `c2pa_context_builder_build` exactly once. From 62f4bbf6a05c223df493aa6363dd1a93102a263c Mon Sep 17 00:00:00 2001 From: tmathern <60901087+tmathern@users.noreply.github.com> Date: Fri, 17 Apr 2026 11:58:56 -0700 Subject: [PATCH 9/9] Bump project version to 0.23.3 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6132bb9..2b8d22d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,7 +14,7 @@ cmake_minimum_required(VERSION 3.27) # This is the current version of this C++ project -project(c2pa-c VERSION 0.23.2) +project(c2pa-c VERSION 0.23.3) # Set the version of the c2pa_rs library used set(C2PA_VERSION "0.80.0")