From 010fd5c13584b9fe901ffe2a2896dc9092c2c4de Mon Sep 17 00:00:00 2001 From: Shubham Singh Date: Fri, 11 Sep 2026 14:48:04 +0530 Subject: [PATCH] fix(core): guard diffusion strategy callbacks --- .../include/rac_diffusion_model_registry.h | 6 +- .../diffusion/rac_diffusion_model_registry.h | 6 +- .../diffusion/diffusion_model_registry.cpp | 22 +- core/tests/CMakeLists.txt | 11 + core/tests/test_diffusion_model_registry.cpp | 201 ++++++++++++++++++ 5 files changed, 240 insertions(+), 6 deletions(-) create mode 100644 core/tests/test_diffusion_model_registry.cpp diff --git a/bindings/swift/Sources/RunAnywhere/CRACommons/include/rac_diffusion_model_registry.h b/bindings/swift/Sources/RunAnywhere/CRACommons/include/rac_diffusion_model_registry.h index 37d36c373c..dcc9e15d21 100644 --- a/bindings/swift/Sources/RunAnywhere/CRACommons/include/rac_diffusion_model_registry.h +++ b/bindings/swift/Sources/RunAnywhere/CRACommons/include/rac_diffusion_model_registry.h @@ -219,11 +219,13 @@ typedef struct rac_diffusion_model_strategy { /** Check if this strategy can handle a model ID */ rac_bool_t (*can_handle)(const char* model_id, void* user_data); - /** Get model definition for a model ID */ + /** Get model definition for a model ID. Return RAC_ERROR_NOT_FOUND to defer to other + * strategies; any other error aborts the registry call and is returned to the caller. */ rac_result_t (*get_model_def)(const char* model_id, rac_diffusion_model_def_t* out_def, void* user_data); - /** Get all models supported by this strategy */ + /** Get all models supported by this strategy. Return RAC_ERROR_NOT_FOUND to defer to other + * strategies; any other error aborts the registry call and is returned to the caller. */ rac_result_t (*list_models)(rac_diffusion_model_def_t** out_models, size_t* out_count, void* user_data); diff --git a/core/include/rac/features/diffusion/rac_diffusion_model_registry.h b/core/include/rac/features/diffusion/rac_diffusion_model_registry.h index fa5523ec61..9cbdca06b5 100644 --- a/core/include/rac/features/diffusion/rac_diffusion_model_registry.h +++ b/core/include/rac/features/diffusion/rac_diffusion_model_registry.h @@ -224,11 +224,13 @@ typedef struct rac_diffusion_model_strategy { /** Check if this strategy can handle a model ID */ rac_bool_t (*can_handle)(const char* model_id, void* user_data); - /** Get model definition for a model ID */ + /** Get model definition for a model ID. Return RAC_ERROR_NOT_FOUND to defer to other + * strategies; any other error aborts the registry call and is returned to the caller. */ rac_result_t (*get_model_def)(const char* model_id, rac_diffusion_model_def_t* out_def, void* user_data); - /** Get all models supported by this strategy */ + /** Get all models supported by this strategy. Return RAC_ERROR_NOT_FOUND to defer to other + * strategies; any other error aborts the registry call and is returned to the caller. */ rac_result_t (*list_models)(rac_diffusion_model_def_t** out_models, size_t* out_count, void* user_data); diff --git a/core/src/features/diffusion/diffusion_model_registry.cpp b/core/src/features/diffusion/diffusion_model_registry.cpp index 727cdf88e9..8129f8f9c9 100644 --- a/core/src/features/diffusion/diffusion_model_registry.cpp +++ b/core/src/features/diffusion/diffusion_model_registry.cpp @@ -352,7 +352,7 @@ rac_result_t rac_diffusion_model_registry_unregister(const char* name) { } rac_result_t rac_diffusion_model_registry_get(const char* model_id, - rac_diffusion_model_def_t* out_def) { + rac_diffusion_model_def_t* out_def) try { if (!model_id || !out_def) { return RAC_ERROR_INVALID_ARGUMENT; } @@ -383,6 +383,12 @@ rac_result_t rac_diffusion_model_registry_get(const char* model_id, RAC_LOG_WARNING(LOG_CAT, "Model not found: %s", model_id); return RAC_ERROR_NOT_FOUND; +} catch (const std::bad_alloc&) { + return RAC_ERROR_OUT_OF_MEMORY; +} catch (...) { + RAC_LOG_ERROR(LOG_CAT, "Strategy callback threw while resolving model '%s'", + model_id ? model_id : "(null)"); + return RAC_ERROR_INTERNAL; } rac_result_t rac_diffusion_model_registry_list(rac_diffusion_model_def_t** out_models, @@ -483,7 +489,7 @@ rac_result_t rac_diffusion_model_registry_list(rac_diffusion_model_def_t** out_m return RAC_ERROR_INTERNAL; } -rac_diffusion_backend_t rac_diffusion_model_registry_select_backend(const char* model_id) { +rac_diffusion_backend_t rac_diffusion_model_registry_select_backend(const char* model_id) try { rac_diffusion_model_def_t model_def; rac_result_t result = rac_diffusion_model_registry_get(model_id, &model_def); @@ -512,6 +518,18 @@ rac_diffusion_backend_t rac_diffusion_model_registry_select_backend(const char* // Return model's preferred backend return model_def.backend; +} catch (const std::bad_alloc&) { + RAC_LOG_ERROR(LOG_CAT, + "Strategy callback threw while selecting a backend for '%s' (result %d), using " + "CoreML (Apple only)", + model_id ? model_id : "(null)", static_cast(RAC_ERROR_OUT_OF_MEMORY)); + return RAC_DIFFUSION_BACKEND_COREML; +} catch (...) { + RAC_LOG_ERROR(LOG_CAT, + "Strategy callback threw while selecting a backend for '%s' (result %d), using " + "CoreML (Apple only)", + model_id ? model_id : "(null)", static_cast(RAC_ERROR_INTERNAL)); + return RAC_DIFFUSION_BACKEND_COREML; } rac_bool_t rac_diffusion_model_registry_is_available(const char* model_id) { diff --git a/core/tests/CMakeLists.txt b/core/tests/CMakeLists.txt index f9d21d96ce..58870bf09f 100644 --- a/core/tests/CMakeLists.txt +++ b/core/tests/CMakeLists.txt @@ -566,6 +566,17 @@ rac_link_archive_deps(test_plugin_registry_isolation) target_compile_features(test_plugin_registry_isolation PRIVATE cxx_std_17) add_test(NAME plugin_registry_isolation_tests COMMAND test_plugin_registry_isolation) +# --- Diffusion model strategy error/exception contract --------------------- +add_executable(test_diffusion_model_registry test_diffusion_model_registry.cpp) +target_include_directories(test_diffusion_model_registry PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} + ${CMAKE_SOURCE_DIR}/include +) +target_link_libraries(test_diffusion_model_registry PRIVATE rac_commons) +rac_link_archive_deps(test_diffusion_model_registry) +target_compile_features(test_diffusion_model_registry PRIVATE cxx_std_20) +add_test(NAME diffusion_model_registry_tests COMMAND test_diffusion_model_registry) + # --- Shared C ABI proto-byte buffer ownership tests ------------------------- add_executable(test_proto_buffer test_proto_buffer.cpp) target_include_directories(test_proto_buffer PRIVATE diff --git a/core/tests/test_diffusion_model_registry.cpp b/core/tests/test_diffusion_model_registry.cpp new file mode 100644 index 0000000000..a8d097a945 --- /dev/null +++ b/core/tests/test_diffusion_model_registry.cpp @@ -0,0 +1,201 @@ +/** + * @file test_diffusion_model_registry.cpp + * @brief Verifies diffusion strategy failures cannot escape or be masked by the C registry API. + */ + +#include +#include +#include +#include +#include + +#include "rac/core/rac_error.h" +#include "rac/features/diffusion/rac_diffusion_model_registry.h" + +namespace { + +int test_count = 0; +int fail_count = 0; + +#define CHECK(condition, label) \ + do { \ + ++test_count; \ + if (!(condition)) { \ + ++fail_count; \ + std::fprintf(stderr, " FAIL: %s (%s:%d)\n", label, __FILE__, __LINE__); \ + } \ + } while (0) + +constexpr const char* kFailingModelId = "failing-model"; +constexpr const char* kThrowCanHandleInternal = "throw-can-handle-internal"; +constexpr const char* kThrowCanHandleOutOfMemory = "throw-can-handle-out-of-memory"; +constexpr const char* kThrowGetInternal = "throw-get-internal"; +constexpr const char* kThrowGetOutOfMemory = "throw-get-out-of-memory"; +constexpr const char* kThrowSelectInternal = "throw-select-internal"; +constexpr const char* kThrowSelectOutOfMemory = "throw-select-out-of-memory"; +constexpr const char* kThrowSelectCanHandle = "throw-select-can-handle"; + +enum class ListBehavior { + kSuccess, + kThrowInternal, + kThrowOutOfMemory, +}; + +struct ThrowingStrategyState { + ListBehavior list_behavior = ListBehavior::kSuccess; + int select_can_handle_calls = 0; +}; + +rac_bool_t failing_can_handle(const char* model_id, void*) { + return std::strcmp(model_id, kFailingModelId) == 0 ? RAC_TRUE : RAC_FALSE; +} + +rac_result_t failing_get_model_def(const char*, rac_diffusion_model_def_t*, void*) { + return RAC_ERROR_NOT_INITIALIZED; +} + +rac_result_t failing_list_models(rac_diffusion_model_def_t**, size_t*, void*) { + return RAC_ERROR_NOT_INITIALIZED; +} + +rac_bool_t throwing_can_handle(const char* model_id, void* user_data) { + auto* state = static_cast(user_data); + if (std::strcmp(model_id, kThrowCanHandleInternal) == 0) { + throw std::runtime_error("can_handle failed"); + } + if (std::strcmp(model_id, kThrowCanHandleOutOfMemory) == 0) { + throw std::bad_alloc(); + } + if (std::strcmp(model_id, kThrowSelectCanHandle) == 0 && + ++state->select_can_handle_calls == 2) { + throw std::runtime_error("can_handle failed during backend selection"); + } + return RAC_TRUE; +} + +rac_result_t throwing_get_model_def(const char* model_id, rac_diffusion_model_def_t* out_def, + void*) { + if (std::strcmp(model_id, kThrowGetInternal) == 0) { + throw std::runtime_error("get_model_def failed"); + } + if (std::strcmp(model_id, kThrowGetOutOfMemory) == 0) { + throw std::bad_alloc(); + } + + *out_def = {}; + out_def->model_id = model_id; + out_def->backend = RAC_DIFFUSION_BACKEND_ONNX; + out_def->platforms = RAC_DIFFUSION_PLATFORM_ALL; + return RAC_SUCCESS; +} + +rac_result_t throwing_list_models(rac_diffusion_model_def_t**, size_t*, void* user_data) { + const auto* state = static_cast(user_data); + if (state->list_behavior == ListBehavior::kThrowOutOfMemory) { + throw std::bad_alloc(); + } + if (state->list_behavior == ListBehavior::kThrowInternal) { + throw std::runtime_error("list_models failed"); + } + return RAC_ERROR_NOT_FOUND; +} + +rac_diffusion_backend_t throwing_select_backend(const rac_diffusion_model_def_t* model, void*) { + if (std::strcmp(model->model_id, kThrowSelectOutOfMemory) == 0) { + throw std::bad_alloc(); + } + if (std::strcmp(model->model_id, kThrowSelectInternal) == 0) { + throw std::runtime_error("select_backend failed"); + } + return RAC_DIFFUSION_BACKEND_ONNX; +} + +void test_returned_strategy_error() { + rac_diffusion_model_registry_cleanup(); + const rac_diffusion_model_strategy_t strategy = { + .name = "Failing", + .can_handle = failing_can_handle, + .get_model_def = failing_get_model_def, + .list_models = failing_list_models, + .select_backend = nullptr, + .load_model = nullptr, + .user_data = nullptr, + }; + CHECK(rac_diffusion_model_registry_register(&strategy) == RAC_SUCCESS, + "failing strategy registers"); + + rac_diffusion_model_def_t model{}; + CHECK(rac_diffusion_model_registry_get(kFailingModelId, &model) == RAC_ERROR_NOT_INITIALIZED, + "get preserves a non-NOT_FOUND strategy error"); + + auto* models = &model; + size_t count = 1; + CHECK(rac_diffusion_model_registry_list(&models, &count) == RAC_ERROR_NOT_INITIALIZED, + "list preserves a non-NOT_FOUND strategy error"); + CHECK(models == nullptr && count == 0, "failed list leaves empty outputs"); + CHECK(rac_diffusion_model_registry_select_backend(kFailingModelId) == + RAC_DIFFUSION_BACKEND_COREML, + "failed lookup keeps the CoreML backend fallback"); +} + +void test_throwing_strategy() { + rac_diffusion_model_registry_cleanup(); + ThrowingStrategyState state; + const rac_diffusion_model_strategy_t strategy = { + .name = "Throwing", + .can_handle = throwing_can_handle, + .get_model_def = throwing_get_model_def, + .list_models = throwing_list_models, + .select_backend = throwing_select_backend, + .load_model = nullptr, + .user_data = &state, + }; + CHECK(rac_diffusion_model_registry_register(&strategy) == RAC_SUCCESS, + "throwing strategy registers"); + + rac_diffusion_model_def_t model{}; + CHECK(rac_diffusion_model_registry_get(kThrowCanHandleOutOfMemory, &model) == + RAC_ERROR_OUT_OF_MEMORY, + "get maps can_handle bad_alloc"); + CHECK(rac_diffusion_model_registry_get(kThrowCanHandleInternal, &model) == RAC_ERROR_INTERNAL, + "get maps unexpected can_handle exception"); + CHECK(rac_diffusion_model_registry_get(kThrowGetOutOfMemory, &model) == + RAC_ERROR_OUT_OF_MEMORY, + "get maps get_model_def bad_alloc"); + CHECK(rac_diffusion_model_registry_get(kThrowGetInternal, &model) == RAC_ERROR_INTERNAL, + "get maps unexpected get_model_def exception"); + + rac_diffusion_model_def_t* models = nullptr; + size_t count = 0; + state.list_behavior = ListBehavior::kThrowOutOfMemory; + CHECK(rac_diffusion_model_registry_list(&models, &count) == RAC_ERROR_OUT_OF_MEMORY, + "list maps list_models bad_alloc"); + CHECK(models == nullptr && count == 0, "bad_alloc list leaves empty outputs"); + state.list_behavior = ListBehavior::kThrowInternal; + CHECK(rac_diffusion_model_registry_list(&models, &count) == RAC_ERROR_INTERNAL, + "list maps unexpected list_models exception"); + CHECK(models == nullptr && count == 0, "unexpected list failure leaves empty outputs"); + + CHECK(rac_diffusion_model_registry_select_backend(kThrowSelectOutOfMemory) == + RAC_DIFFUSION_BACKEND_COREML, + "select_backend maps bad_alloc to the CoreML fallback"); + CHECK(rac_diffusion_model_registry_select_backend(kThrowSelectInternal) == + RAC_DIFFUSION_BACKEND_COREML, + "select_backend maps unexpected exception to the CoreML fallback"); + state.select_can_handle_calls = 0; + CHECK(rac_diffusion_model_registry_select_backend(kThrowSelectCanHandle) == + RAC_DIFFUSION_BACKEND_COREML, + "select_backend contains a can_handle exception"); +} + +} // namespace + +int main() { + std::fprintf(stdout, "test_diffusion_model_registry\n"); + test_returned_strategy_error(); + test_throwing_strategy(); + rac_diffusion_model_registry_cleanup(); + + std::fprintf(stdout, "\n%d checks, %d failed\n", test_count, fail_count); + return fail_count == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +}