Skip to content

Improved_ref_count_check #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edwardbr merged 2 commits into from
Feb 20, 2026
Merged

Improved_ref_count_check #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
caa3be3
Improved reference count checks
edwardbr Feb 20, 2026
cfbd670
changed overeager assert on error check into a config option
edwardbr Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
},
{
Expand All @@ -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"
}
},
{
Expand Down
16 changes: 16 additions & 0 deletions cmake/Canopy.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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}
Expand Down
21 changes: 20 additions & 1 deletion rpc/include/rpc/internal/logger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...)
Expand Down
84 changes: 27 additions & 57 deletions rpc/include/rpc/internal/remote_pointer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<rpc::casting_interface*>(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

Expand All @@ -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

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading