Skip to content

Commit

Permalink
Create switch for disabling callback logging
Browse files Browse the repository at this point in the history
We're seeing reports of folks running into ODR issues when linking
together TUs built with and without `NDEBUG` defined. The reason is that
`unifex::inplace_stop_callback_base`'s size and layout depend on whether
`NDEBUG` is defined, leading to incompatible ABIs between the two build
modes.

This change introduces a new preprocessor macro,
`UNIFEX_LOG_DANGLING_STOP_CALLBACKS` that controls whether
`inplace_stop_callback_base` knows a stringified typename of the
deriving class and wheter `inplace_stop_source` uses that knowledge to
log the names of dangling stop callbacks in its destructor. I've also
updated `config.hpp.in` to retain existing behaviour: the default state
for `UNIFEX_LOG_DANGLING_STOP_CALLBACKS` is on when `NDEBUG` is not
defined an off when `NDEBUG` is defined. The defaults can be overridden.
  • Loading branch information
ispeters committed Mar 15, 2024
1 parent 75d4692 commit 4b0d84a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
10 changes: 10 additions & 0 deletions include/unifex/config.hpp.in
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,13 @@
#else
#define UNIFEX_DEPRECATED_HEADER(MSG) /**/
#endif

#if !defined(UNIFEX_LOG_DANGLING_STOP_CALLBACKS)
# if !defined(NDEBUG)
// logging not already specified and this is a debug build so default on
# define UNIFEX_LOG_DANGLING_STOP_CALLBACKS 1
# else
// logging not already specified and this is a release build so default off
# define UNIFEX_LOG_DANGLING_STOP_CALLBACKS 0
# endif
#endif
14 changes: 10 additions & 4 deletions include/unifex/inplace_stop_token.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,24 @@ class inplace_stop_token;
template <typename F>
class inplace_stop_callback;

#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
// defining UNIFEX_LOG_DANGLING_STOP_CALLBACKS changes the ABI so let's turn
// that into a link-time error instead of a runtime ODR issue
# define inplace_stop_callback_base inplace_stop_callback_base_d
#endif

class inplace_stop_callback_base {
public:
void execute() noexcept { this->execute_(this); }

#ifndef NDEBUG
#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
char const* type_name() const noexcept { return type_name_; }
#endif

protected:
using execute_fn = void(inplace_stop_callback_base* cb) noexcept;

#ifndef NDEBUG
#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
explicit inplace_stop_callback_base(
inplace_stop_source* source,
execute_fn* execute,
Expand All @@ -72,7 +78,7 @@ class inplace_stop_callback_base {
inplace_stop_callback_base** prevPtr_ = nullptr;
bool* removedDuringCallback_ = nullptr;
std::atomic<bool> callbackCompleted_{false};
#ifndef NDEBUG
#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
char const* type_name_ = nullptr;
#endif
};
Expand Down Expand Up @@ -174,7 +180,7 @@ class inplace_stop_callback final : private inplace_stop_callback_base {
explicit inplace_stop_callback(
inplace_stop_token token,
T&& func) noexcept(std::is_nothrow_constructible_v<F, T>)
#ifndef NDEBUG
#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
: inplace_stop_callback_base(
token.source_,
&inplace_stop_callback::execute_impl,
Expand Down
6 changes: 4 additions & 2 deletions source/inplace_stop_token.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@

#include <unifex/spin_wait.hpp>

#ifndef NDEBUG
#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
# include <stdio.h>
#endif

namespace unifex {

inplace_stop_source::~inplace_stop_source() {
UNIFEX_ASSERT((state_.load(std::memory_order_relaxed) & locked_flag) == 0);
#ifndef NDEBUG
#if UNIFEX_LOG_DANGLING_STOP_CALLBACKS
// consume the last writes made under the lock
(void)state_.load(std::memory_order_acquire);
for (auto* cb = callbacks_; cb != nullptr; cb = cb->next_) {
printf("dangling inplace_stop_callback: %s\n", cb->type_name());
fflush(stdout);
Expand Down

0 comments on commit 4b0d84a

Please sign in to comment.