From: Kefu Chai Date: Sun, 14 Jun 2026 06:52:00 +0000 (+0800) Subject: journal/ObjectPlayer: don't acquire locks in destructor X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a35a92639f5dc8d6a98415711242f2491fb45f46;p=ceph.git journal/ObjectPlayer: don't acquire locks in destructor ~ObjectPlayer took m_timer_lock only to assert two invariants. but that lock is borrowed by reference from the caller's SafeTimer, and an ObjectPlayer can outlive it: a C_Fetch/C_WatchFetch completion on the librados finisher may hold the last reference and run ~ObjectPlayer after the timer and its lock are already gone. re-taking the freed lock is a heap-use-after-free, which unittest_journal hits on arm64 under ASan: ==ERROR: AddressSanitizer: heap-use-after-free journal::ObjectPlayer::~ObjectPlayer ObjectPlayer.cc:63 journal::ObjectPlayer::C_WatchFetch::~C_WatchFetch journal::ObjectPlayer::C_Fetch::finish the lock isn't needed though: at refcount 0 the watch has been cancelled (m_watch_ctx == nullptr, asserted below) so no timer task references us, and no fetch is in flight since a pending fetch holds a reference. nothing else can touch our state. Furthermore, we can also skip acquiring `m_lock` as well, because, in the destructor, it shouldn't really matter -- if one of these asserts fails because the execution of the destructor races with some `ObjectPlayer` mthod, we would get what the `assert()` was added for. They are here to catch bugs and such a race just being possible is a bug in itself. Fixes: https://tracker.ceph.com/issues/77496 Signed-off-by: Kefu Chai --- diff --git a/src/journal/ObjectPlayer.cc b/src/journal/ObjectPlayer.cc index 29893cec2d9..e01355e2f72 100644 --- a/src/journal/ObjectPlayer.cc +++ b/src/journal/ObjectPlayer.cc @@ -59,12 +59,8 @@ ObjectPlayer::ObjectPlayer(librados::IoCtx &ioctx, } ObjectPlayer::~ObjectPlayer() { - { - std::lock_guard timer_locker{m_timer_lock}; - std::lock_guard locker{m_lock}; - ceph_assert(!m_fetch_in_progress); - ceph_assert(m_watch_ctx == nullptr); - } + ceph_assert(!m_fetch_in_progress); + ceph_assert(m_watch_ctx == nullptr); } void ObjectPlayer::fetch(Context *on_finish) {