diff --git a/score/launch_manager/daemon/src/common/concurrency/BUILD b/score/launch_manager/daemon/src/common/concurrency/BUILD index e2d8ae86..3f1550f4 100644 --- a/score/launch_manager/daemon/src/common/concurrency/BUILD +++ b/score/launch_manager/daemon/src/common/concurrency/BUILD @@ -49,11 +49,6 @@ cc_library( cc_test( name = "mpmc_concurrent_queue_test", srcs = ["mpmc_concurrent_queue_test.cpp"], - # TODO(eclipse-score/lifecycle#241): UBSan reports a misaligned reference while this - # test fixture is constructed on ASan's fake stack; the queue itself uses - # 64-byte-aligned members, so the report is a fake-stack alignment false - # positive rather than a queue bug. - tags = ["no-asan"], visibility = ["//tests:__subpackages__"], deps = [ ":mpmc_concurrent_queue", diff --git a/score/launch_manager/daemon/src/common/concurrency/mpmc_concurrent_queue.hpp b/score/launch_manager/daemon/src/common/concurrency/mpmc_concurrent_queue.hpp index 41684cf6..751bdefe 100644 --- a/score/launch_manager/daemon/src/common/concurrency/mpmc_concurrent_queue.hpp +++ b/score/launch_manager/daemon/src/common/concurrency/mpmc_concurrent_queue.hpp @@ -273,22 +273,36 @@ class MPMCConcurrentQueue return score::cpp::blank{}; } + /// @brief Helper type to align members to the cache lines. + /// @details Using padding rather than alignas(CacheLineSize) so that + /// ASan's fake stack doesn't cause UBSan misalignment. + template + struct CacheLinePaddedAtomic : public std::atomic + { + static_assert(sizeof(std::atomic) < CacheLineSize, + "atomic is too large to pad to one cache line"); + using std::atomic::atomic; + + private: + char _pad[CacheLineSize - sizeof(std::atomic)]{}; + }; + /// @brief Underlying storage. std::array m_slots; /// @brief The front of the queue; claimed by consumers via fetch_add in pop. /// @details Aligned so that m_head and m_tail do not share a cache line. - alignas(CacheLineSize) std::atomic m_head{0}; + CacheLinePaddedAtomic m_head{0}; /// @brief The back of the queue; claimed by producers via fetch_add in push_impl. /// @details Aligned so that m_head and m_tail do not share a cache line. - alignas(CacheLineSize) std::atomic m_tail{0}; + CacheLinePaddedAtomic m_tail{0}; /// @brief Set to true by stop(); causes push() to return false and pop() to /// return std::nullopt instead of blocking. /// @details Aligned on its own cache line so that the single stop() write /// does not cause false sharing with m_tail updates in push_impl(). - alignas(CacheLineSize) std::atomic m_stopped{false}; + CacheLinePaddedAtomic m_stopped{false}; /// @brief Counts items currently in the queue; consumers block on this when /// the queue is empty.