diff --git a/CMakePresets.json b/CMakePresets.json index 2ad7428..2897e89 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -40,7 +40,9 @@ "CANOPY_DEBUG_ALL": "OFF", "CANOPY_ENABLE_COVERAGE": "OFF", "CANOPY_ENABLE_CLANG_TIDY": "OFF", - "CANOPY_ENABLE_CLANG_TIDY_FIX": "OFF" + "CANOPY_ENABLE_CLANG_TIDY_FIX": "OFF", + "CANOPY_ADD_REF_COUNT_CHECKS": "OFF", + "CANOPY_ASSERT_ON_LOGGER_ERROR": "OFF" } }, { @@ -55,7 +57,9 @@ "CANOPY_USE_TELEMETRY": "ON", "CANOPY_USE_CONSOLE_TELEMETRY": "ON", "CANOPY_USE_TELEMETRY_RAII_LOGGING": "ON", - "CANOPY_DEBUG_DEFAULT_DESTRUCTOR": "ON" + "CANOPY_DEBUG_DEFAULT_DESTRUCTOR": "ON", + "CANOPY_ADD_REF_COUNT_CHECKS": "ON", + "CANOPY_ASSERT_ON_LOGGER_ERROR": "ON" } }, { diff --git a/cmake/Canopy.cmake b/cmake/Canopy.cmake index ce06619..e882088 100644 --- a/cmake/Canopy.cmake +++ b/cmake/Canopy.cmake @@ -60,6 +60,8 @@ if(NOT DEPENDENCIES_LOADED) # Logging and Telemetry Options # #################################################################################################################### option(CANOPY_USE_LOGGING "Turn on Canopy logging" OFF) + option(CANOPY_ADD_REF_COUNT_CHECKS "Turn on Canopy refcount checks" OFF) + option(CANOPY_ASSERT_ON_LOGGER_ERROR "Turn on asserts when there is a logger error") option(CANOPY_USE_THREAD_LOCAL_LOGGING "Turn on thread-local circular buffer logging" OFF) option(CANOPY_HANG_ON_FAILED_ASSERT "Hang on failed assert" OFF) option(CANOPY_USE_TELEMETRY "Turn on Canopy telemetry" OFF) @@ -210,6 +212,18 @@ if(NOT DEPENDENCIES_LOADED) set(CANOPY_USE_LOGGING_FLAG) endif() + if(CANOPY_ADD_REF_COUNT_CHECKS) + set(CANOPY_ADD_REF_COUNT_CHECKS_FLAG CANOPY_ADD_REF_COUNT_CHECKS) + else() + set(CANOPY_ADD_REF_COUNT_CHECKS_FLAG) + endif() + + if(CANOPY_ASSERT_ON_LOGGER_ERROR) + set(CANOPY_ASSERT_ON_LOGGER_ERROR_FLAG CANOPY_ASSERT_ON_LOGGER_ERROR) + else() + set(CANOPY_ASSERT_ON_LOGGER_ERROR_FLAG) + endif() + if(CANOPY_USE_THREAD_LOCAL_LOGGING) set(CANOPY_USE_THREAD_LOCAL_LOGGING_FLAG CANOPY_USE_THREAD_LOCAL_LOGGING) else() @@ -262,6 +276,8 @@ if(NOT DEPENDENCIES_LOADED) NOMINMAX _SILENCE_ALL_MS_EXT_DEPRECATION_WARNINGS ${CANOPY_USE_LOGGING_FLAG} + ${CANOPY_ADD_REF_COUNT_CHECKS_FLAG} + ${CANOPY_ASSERT_ON_LOGGER_ERROR_FLAG} ${CANOPY_USE_THREAD_LOCAL_LOGGING_FLAG} ${CANOPY_BUILD_COROUTINE_FLAG} ${CANOPY_HANG_ON_FAILED_ASSERT_FLAG} diff --git a/rpc/include/rpc/internal/logger.h b/rpc/include/rpc/internal/logger.h index ce861cc..e9350c9 100644 --- a/rpc/include/rpc/internal/logger.h +++ b/rpc/include/rpc/internal/logger.h @@ -86,20 +86,39 @@ extern "C" RPC_LOG_BACKEND(3, formatted); \ } while (0) +#ifdef CANOPY_ASSERT_ON_LOGGER_ERROR #define RPC_ERROR(format_str, ...) \ do \ { \ + RPC_ASSERT(false); \ auto formatted = fmt::format(format_str, ##__VA_ARGS__); \ RPC_LOG_BACKEND(4, formatted); \ } while (0) +#else +#define RPC_ERROR(format_str, ...) \ + do \ + { \ + auto formatted = fmt::format(format_str, ##__VA_ARGS__); \ + RPC_LOG_BACKEND(4, formatted); \ + } while (0) +#endif +#ifdef CANOPY_ASSERT_ON_LOGGER_ERROR #define RPC_CRITICAL(format_str, ...) \ do \ { \ + RPC_ASSERT(false); \ auto formatted = fmt::format(format_str, ##__VA_ARGS__); \ RPC_LOG_BACKEND(5, formatted); \ } while (0) - +#else +#define RPC_CRITICAL(format_str, ...) \ + do \ + { \ + auto formatted = fmt::format(format_str, ##__VA_ARGS__); \ + RPC_LOG_BACKEND(5, formatted); \ + } while (0) +#endif #else // Disabled logging - all macros are no-ops #define RPC_DEBUG(format_str, ...) diff --git a/rpc/include/rpc/internal/remote_pointer.h b/rpc/include/rpc/internal/remote_pointer.h index 4449b89..2528535 100644 --- a/rpc/include/rpc/internal/remote_pointer.h +++ b/rpc/include/rpc/internal/remote_pointer.h @@ -2339,49 +2339,35 @@ namespace rpc else { // Remote object: establish optimistic reference (0→1 transition, async!) -#ifdef CANOPY_USE_LOGGING - // TELEMETRY: Check reference counts BEFORE establishing optimistic reference - long cb_shared_before = cb->shared_count_.load(std::memory_order_acquire); +#ifdef CANOPY_ADD_REF_COUNT_CHECKS + // TELEMETRY: shared_count is not touched by make_optimistic, so only check optimistic. + // optimistic_count fires a remote add_ref only on the 0→1 transition, so cb_optimistic + // can exceed inherited_optimistic when multiple local copies exist. The invariant is + // that they agree on whether a remote reference exists (both zero or both non-zero). long cb_optimistic_before = cb->optimistic_count_.load(std::memory_order_acquire); - // Get object_proxy to check inherited counts (service-level stub counts) auto casting_iface = reinterpret_cast(in.internal_get_ptr()); auto obj_proxy = casting_iface->__rpc_get_object_proxy(); - int inherited_shared_before = 0; int inherited_optimistic_before = 0; if (obj_proxy) { + int unused_shared = 0; __rpc_internal::__shared_ptr_control_block::get_object_proxy_reference_counts( - obj_proxy, inherited_shared_before, inherited_optimistic_before); + obj_proxy, unused_shared, inherited_optimistic_before); } - RPC_DEBUG("make_optimistic(shared_ptr→optimistic_ptr): BEFORE - control_block(shared={}, optimistic={}), " - "object_proxy(inherited_shared={}, inherited_optimistic={})", - cb_shared_before, + RPC_DEBUG("make_optimistic(shared_ptr→optimistic_ptr): BEFORE - control_block(optimistic={}), " + "object_proxy(inherited_optimistic={})", cb_optimistic_before, - inherited_shared_before, inherited_optimistic_before); - // NOTE: It's EXPECTED that before make_optimistic, we have shared=1, optimistic=0 - // This is the normal state when converting a shared_ptr to optimistic_ptr - // The validation below checks this expected state - - // Verify control block shared count matches object proxy shared count - if (obj_proxy && cb_shared_before != inherited_shared_before) + if (obj_proxy && (cb_optimistic_before == 0) != (inherited_optimistic_before == 0)) { - RPC_ERROR( - "make_optimistic: Control block shared count ({}) doesn't match object_proxy shared count ({})", - cb_shared_before, - inherited_shared_before); - } - - // Verify control block optimistic count matches object proxy optimistic count - if (obj_proxy && cb_optimistic_before != inherited_optimistic_before) - { - RPC_ERROR("make_optimistic: Control block optimistic count ({}) doesn't match object_proxy optimistic " - "count ({})", + RPC_ERROR("make_optimistic: Control block optimistic count ({}) and object_proxy optimistic " + "count ({}) disagree on whether a remote reference exists", cb_optimistic_before, inherited_optimistic_before); + RPC_ASSERT(false); } #endif @@ -2391,50 +2377,34 @@ namespace rpc CO_RETURN err; // Failed to establish remote reference } -#ifdef CANOPY_USE_LOGGING - // TELEMETRY: Check reference counts AFTER establishing optimistic reference - long cb_shared_after = cb->shared_count_.load(std::memory_order_acquire); +#ifdef CANOPY_ADD_REF_COUNT_CHECKS long cb_optimistic_after = cb->optimistic_count_.load(std::memory_order_acquire); - int inherited_shared_after = 0; int inherited_optimistic_after = 0; if (obj_proxy) { + int unused_shared = 0; __rpc_internal::__shared_ptr_control_block::get_object_proxy_reference_counts( - obj_proxy, inherited_shared_after, inherited_optimistic_after); + obj_proxy, unused_shared, inherited_optimistic_after); } - RPC_DEBUG("make_optimistic(shared_ptr→optimistic_ptr): AFTER - control_block(shared={}, optimistic={}), " - "object_proxy(inherited_shared={}, inherited_optimistic={})", - cb_shared_after, + RPC_DEBUG("make_optimistic(shared_ptr→optimistic_ptr): AFTER - control_block(optimistic={}), " + "object_proxy(inherited_optimistic={})", cb_optimistic_after, - inherited_shared_after, inherited_optimistic_after); - // After make_optimistic, we expect shared=1, optimistic=1 (both counts should be positive) - if (cb_shared_after == 0 || cb_optimistic_after == 0) + if (cb_optimistic_after == 0) { - RPC_ERROR("make_optimistic: Control block count zero AFTER increment - shared={} optimistic={} (both " - "should be > 0)", - cb_shared_after, - cb_optimistic_after); - } - - // Verify control block shared count matches object proxy shared count - if (obj_proxy && cb_shared_after != inherited_shared_after) - { - RPC_ERROR("make_optimistic: Control block shared count ({}) doesn't match object_proxy shared count " - "({}) AFTER", - cb_shared_after, - inherited_shared_after); + RPC_ERROR("make_optimistic: Control block optimistic count is zero AFTER increment"); + RPC_ASSERT(false); } - // Verify control block optimistic count matches object proxy optimistic count - if (obj_proxy && cb_optimistic_after != inherited_optimistic_after) + if (obj_proxy && (cb_optimistic_after == 0) != (inherited_optimistic_after == 0)) { - RPC_ERROR("make_optimistic: Control block optimistic count ({}) doesn't match object_proxy optimistic " - "count ({}) AFTER", + RPC_ERROR("make_optimistic: Control block optimistic count ({}) and object_proxy optimistic " + "count ({}) disagree on whether a remote reference exists AFTER increment", cb_optimistic_after, inherited_optimistic_after); + RPC_ASSERT(false); } #endif @@ -2467,7 +2437,7 @@ namespace rpc else { // Remote object: establish optimistic reference (0→1 transition, async!) -#ifdef CANOPY_USE_LOGGING +#ifdef CANOPY_ADD_REF_COUNT_CHECKS // TELEMETRY: Check reference counts BEFORE establishing optimistic reference long cb_shared_before = cb->shared_count_.load(std::memory_order_acquire); long cb_optimistic_before = cb->optimistic_count_.load(std::memory_order_acquire); @@ -2515,7 +2485,7 @@ namespace rpc CO_RETURN err; // Failed (likely OBJECT_GONE) } -#ifdef CANOPY_USE_LOGGING +#ifdef CANOPY_ADD_REF_COUNT_CHECKS // TELEMETRY: Check reference counts AFTER establishing optimistic reference long cb_shared_after = cb->shared_count_.load(std::memory_order_acquire); long cb_optimistic_after = cb->optimistic_count_.load(std::memory_order_acquire);