fix: Make sure progress callback lives long enough

include/c2pa.hpp

@@ -112,6 +112,56 @@ namespace c2pa
         std::string message_;
     };
 
+    /// @brief Phase values reported to the ProgressCallbackFunc.
+    ///
+    /// @details A scoped C++ mirror of `C2paProgressPhase` from c2pa.h.
+    ///          Values are verified at compile time to match the C enum, so any
+    ///          future divergence in c2pa-rs will be caught as a build error.
+    ///
+    ///          Phases emitted during a typical sign cycle (in order):
+    ///            AddingIngredient → Thumbnail → Hashing → Signing → Embedding →
+    ///            (if verify_after_sign) VerifyingManifest → VerifyingSignature →
+    ///            VerifyingAssetHash → VerifyingIngredient
+    ///
+    ///          Phases emitted during reading:
+    ///            Reading → VerifyingManifest → VerifyingSignature →
+    ///            VerifyingAssetHash → VerifyingIngredient
+    enum class ProgressPhase : uint8_t {
+        Reading               = 0,
+        VerifyingManifest     = 1,
+        VerifyingSignature    = 2,
+        VerifyingIngredient   = 3,
+        VerifyingAssetHash    = 4,
+        AddingIngredient      = 5,
+        Thumbnail             = 6,
+        Hashing               = 7,
+        Signing               = 8,
+        Embedding             = 9,
+        FetchingRemoteManifest = 10,
+        Writing               = 11,
+        FetchingOCSP          = 12,
+        FetchingTimestamp     = 13,
+    };
+
+    /// @brief Type alias for the progress callback passed to ContextBuilder::with_progress_callback().
+    ///
+    /// @details The callback is invoked at each major phase of signing and reading operations.
+    ///          Returning false from the callback aborts the operation with an
+    ///          OperationCancelled error (equivalent to calling Context::cancel()).
+    ///
+    /// @param phase  Current operation phase.
+    /// @param step   1-based step index within the phase.
+    ///               0 = indeterminate (use as liveness signal); resets to 1 at each new phase.
+    /// @param total  0 = indeterminate; 1 = single-shot; >1 = determinate (step/total = fraction).
+    /// @return true to continue the operation, false to request cancellation.
+    ///
+    /// @note The callback must not throw. If it throws, the implementation catches the
+    ///       exception and reports cancellation to the underlying library (same as returning
+    ///       false); the original exception is not propagated. Prefer returning false or
+    ///       using Context::cancel() instead of throwing.
+    ///
+    using ProgressCallbackFunc = std::function<bool(ProgressPhase phase, uint32_t step, uint32_t total)>;
+
     /// @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).
@@ -165,11 +215,23 @@ namespace c2pa
         /// @warning Implementations must ensure is_valid() == true implies c_context() != nullptr.
         [[nodiscard]] virtual bool is_valid() const noexcept = 0;
 
+        /// @brief Get shared ownership of the progress callback, if any.
+        /// @return shared_ptr to the callback, or empty if none was set.
+        [[nodiscard]] std::shared_ptr<ProgressCallbackFunc> callback_guard() const noexcept {
+            return callback_owner_;
+        }
+
     protected:
         IContextProvider() = default;
 
         IContextProvider(const IContextProvider&) = delete;
         IContextProvider& operator=(const IContextProvider&) = delete;
+
+        /// Shared-owned progress callback. Non-empty only when a progress callback
+        /// has been configured (e.g. via ContextBuilder::with_progress_callback()).
+        /// Builder and Reader copy this at construction to extend the callback's
+        /// lifetime beyond the originating context.
+        std::shared_ptr<ProgressCallbackFunc> callback_owner_;
     };
 
     /// @brief (C2PA SDK) Settings configuration object for creating contexts.
@@ -238,56 +300,6 @@ namespace c2pa
         C2paSettings* settings_ptr;
     };
 
-     /// @brief Phase values reported to the ProgressCallbackFunc.
-    ///
-    /// @details A scoped C++ mirror of `C2paProgressPhase` from c2pa.h.
-    ///          Values are verified at compile time to match the C enum, so any
-    ///          future divergence in c2pa-rs will be caught as a build error.
-    ///
-    ///          Phases emitted during a typical sign cycle (in order):
-    ///            AddingIngredient → Thumbnail → Hashing → Signing → Embedding →
-    ///            (if verify_after_sign) VerifyingManifest → VerifyingSignature →
-    ///            VerifyingAssetHash → VerifyingIngredient
-    ///
-    ///          Phases emitted during reading:
-    ///            Reading → VerifyingManifest → VerifyingSignature →
-    ///            VerifyingAssetHash → VerifyingIngredient
-    enum class ProgressPhase : uint8_t {
-        Reading               = 0,
-        VerifyingManifest     = 1,
-        VerifyingSignature    = 2,
-        VerifyingIngredient   = 3,
-        VerifyingAssetHash    = 4,
-        AddingIngredient      = 5,
-        Thumbnail             = 6,
-        Hashing               = 7,
-        Signing               = 8,
-        Embedding             = 9,
-        FetchingRemoteManifest = 10,
-        Writing               = 11,
-        FetchingOCSP          = 12,
-        FetchingTimestamp     = 13,
-    };
-
-    /// @brief Type alias for the progress callback passed to ContextBuilder::with_progress_callback().
-    ///
-    /// @details The callback is invoked at each major phase of signing and reading operations.
-    ///          Returning false from the callback aborts the operation with an
-    ///          OperationCancelled error (equivalent to calling Context::cancel()).
-    ///
-    /// @param phase  Current operation phase.
-    /// @param step   1-based step index within the phase.
-    ///               0 = indeterminate (use as liveness signal); resets to 1 at each new phase.
-    /// @param total  0 = indeterminate; 1 = single-shot; >1 = determinate (step/total = fraction).
-    /// @return true to continue the operation, false to request cancellation.
-    ///
-    /// @note The callback must not throw. If it throws, the implementation catches the
-    ///       exception and reports cancellation to the underlying library (same as returning
-    ///       false); the original exception is not propagated. Prefer returning false or
-    ///       using Context::cancel() instead of throwing.
-    ///
-    using ProgressCallbackFunc = std::function<bool(ProgressPhase phase, uint32_t step, uint32_t total)>;
-
     /// @brief C2PA context implementing IContextProvider.
     /// @details Context objects manage C2PA SDK configuration and state.
     ///          Contexts can be created via direct construction or the ContextBuilder:
@@ -477,10 +489,6 @@ namespace c2pa
 
     private:
         C2paContext* context;
-
-        /// Heap-owned ProgressCallbackFunc; non-null only when set via
-        /// ContextBuilder::with_progress_callback().  Deleted in the destructor.
-        void* callback_owner_ = nullptr;
     };
 
     /// @brief Get the version of the C2PA library.
@@ -728,6 +736,7 @@ namespace c2pa
         C2paReader *c2pa_reader;
         std::unique_ptr<std::ifstream> owned_stream;       // Owns file stream when created from path
         std::unique_ptr<CppIStream> cpp_stream;            // Wraps stream for C API; destroyed before owned_stream
+        std::shared_ptr<ProgressCallbackFunc> callback_guard_;
 
     public:
         /// @brief Create a Reader from a context and stream.
@@ -782,7 +791,8 @@ namespace c2pa
         Reader(Reader&& other) noexcept
             : c2pa_reader(std::exchange(other.c2pa_reader, nullptr)),
               owned_stream(std::move(other.owned_stream)),
-              cpp_stream(std::move(other.cpp_stream)) {
+              cpp_stream(std::move(other.cpp_stream)),
+              callback_guard_(std::move(other.callback_guard_)) {
         }
 
         Reader& operator=(Reader&& other) noexcept {
@@ -791,6 +801,7 @@ namespace c2pa
                 c2pa_reader = std::exchange(other.c2pa_reader, nullptr);
                 owned_stream = std::move(other.owned_stream);
                 cpp_stream = std::move(other.cpp_stream);
+                callback_guard_ = std::move(other.callback_guard_);
             }
             return *this;
         }
@@ -939,6 +950,7 @@ namespace c2pa
     {
     private:
         C2paBuilder *builder;
+        std::shared_ptr<ProgressCallbackFunc> callback_guard_;
 
     public:
         /// @brief Create a Builder from a context with an empty manifest.
@@ -970,13 +982,16 @@ namespace c2pa
 
         Builder& operator=(const Builder&) = delete;
 
-        Builder(Builder&& other) noexcept : builder(std::exchange(other.builder, nullptr)) {
+        Builder(Builder&& other) noexcept
+            : builder(std::exchange(other.builder, nullptr)),
+              callback_guard_(std::move(other.callback_guard_)) {
         }
 
         Builder& operator=(Builder&& other) noexcept {
             if (this != &other) {
                 c2pa_free(builder);
                 builder = std::exchange(other.builder, nullptr);
+                callback_guard_ = std::move(other.callback_guard_);
             }
             return *this;
         }

src/c2pa_builder.cpp

@@ -41,6 +41,7 @@ namespace c2pa
         if (builder == nullptr) {
             throw C2paException("Failed to create builder from context");
         }
+        callback_guard_ = context.callback_guard();
     }
 
     Builder::Builder(IContextProvider& context, const std::string &manifest_json)
@@ -54,6 +55,7 @@ namespace c2pa
         if (builder == nullptr) {
             throw C2paException("Failed to create builder from context");
         }
+        callback_guard_ = context.callback_guard();
 
         // Apply the manifest definition to the Builder.
         // Note: c2pa_builder_with_definition always consumes the builder pointer,

src/c2pa_context.cpp

@@ -39,23 +39,21 @@ namespace c2pa
     }
 
     Context::Context(Context&& other) noexcept
-        : context(std::exchange(other.context, nullptr)),
-          callback_owner_(std::exchange(other.callback_owner_, nullptr)) {
+        : context(std::exchange(other.context, nullptr)) {
+        callback_owner_ = std::move(other.callback_owner_);
     }
 
     Context& Context::operator=(Context&& other) noexcept {
         if (this != &other) {
             c2pa_free(context);
-            delete static_cast<ProgressCallbackFunc*>(callback_owner_);
             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<ProgressCallbackFunc*>(callback_owner_);
     }
 
     void Context::cancel() noexcept {
@@ -257,9 +255,9 @@ namespace c2pa
         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();
+        // Transfer progress callback heap ownership to the Context.
+        // Using shared_ptr so Builder/Reader can extend the callback's lifetime.
+        result.callback_owner_ = std::move(pending_callback_);
         return result;
     }
 } // namespace c2pa

src/c2pa_reader.cpp

@@ -49,6 +49,7 @@ namespace c2pa
         if (c2pa_reader == nullptr) {
             throw C2paException("Failed to create reader from context");
         }
+        callback_guard_ = context.callback_guard();
 
         cpp_stream = std::make_unique<CppIStream>(stream);
         // Update reader with stream.
@@ -73,6 +74,7 @@ namespace c2pa
         if (c2pa_reader == nullptr) {
             throw C2paException("Failed to create reader from context");
         }
+        callback_guard_ = context.callback_guard();
 
         // Create owned stream that will live as long as the Reader
         owned_stream = std::make_unique<std::ifstream>(source_path, std::ios::binary);

tests/context.test.cpp

@@ -684,3 +684,60 @@ TEST_F(ContextTest, ProgressCallback_SurvivesBuilderMove) {
     EXPECT_NO_THROW(sign_with_progress_context(context, get_temp_path("progress_builder_move.jpg")));
     EXPECT_GT(call_count.load(), 0);
 }
+
+// Context destroyed before Builder is used; callback must still work.
+TEST_F(ContextTest, ProgressCallback_SurvivesContextDestruction_Builder) {
+    std::atomic<int> call_count{0};
+
+    // Build a Builder inside a nested scope so the Context is destroyed first.
+    c2pa::Builder builder = [&]() {
+        auto context = c2pa::Context::ContextBuilder()
+            .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) {
+                ++call_count;
+                return true;
+            })
+            .create_context();
+
+        auto manifest = c2pa_test::read_text_file(c2pa_test::get_fixture_path("training.json"));
+        return c2pa::Builder(context, manifest);
+        // context is destroyed here
+    }();
+
+    auto certs       = c2pa_test::read_text_file(c2pa_test::get_fixture_path("es256_certs.pem"));
+    auto private_key = c2pa_test::read_text_file(c2pa_test::get_fixture_path("es256_private.key"));
+    auto asset_path  = c2pa_test::get_fixture_path("A.jpg");
+    c2pa::Signer signer("es256", certs, private_key);
+
+    // Context is already destroyed; signing must not crash.
+    EXPECT_NO_THROW(builder.sign(asset_path, get_temp_path("callback_survives_builder.jpg"), signer));
+    EXPECT_GT(call_count.load(), 0);
+}
+
+// Context destroyed before Reader is used; callback must still work.
+TEST_F(ContextTest, ProgressCallback_SurvivesContextDestruction_Reader) {
+    // First sign a file so we have something to read.
+    {
+        c2pa::Context sign_ctx;
+        sign_with_progress_context(sign_ctx, get_temp_path("progress_read_survive_src.jpg"));
+    }
+
+    std::atomic<int> call_count{0};
+    auto signed_path = get_temp_path("progress_read_survive_src.jpg");
+
+    // Build a Reader inside a nested scope so the Context is destroyed first.
+    c2pa::Reader reader = [&]() {
+        auto context = c2pa::Context::ContextBuilder()
+            .with_progress_callback([&](c2pa::ProgressPhase, uint32_t, uint32_t) {
+                ++call_count;
+                return true;
+            })
+            .create_context();
+
+        return c2pa::Reader(context, signed_path);
+        // context is destroyed here
+    }();
+
+    // Context is already destroyed; accessing the reader must not crash.
+    EXPECT_NO_THROW((void)reader.json());
+    EXPECT_GT(call_count.load(), 0);
+}