]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commit
test/osd: fix Message and Connection refcount leaks 68614/head
authorKefu Chai <k.chai@proxmox.com>
Sat, 25 Apr 2026 04:43:24 +0000 (12:43 +0800)
committerKefu Chai <k.chai@proxmox.com>
Sat, 25 Apr 2026 13:29:54 +0000 (21:29 +0800)
commitc445bc285cbb23c8e02e3e0388a962d2659d99c8
tree1fd32df0be8800ea13de4b85aeaa97201dacd614
parent5c651b65f21240e7ccfee7f21ea40ef9e653cf6b
test/osd: fix Message and Connection refcount leaks

unittest_backend_basics and unittest_ecfailover_with_peering fail under
ASan with megabytes of leaks per run, all originating in MockMessenger
and the listeners that feed into it.  A representative LSan stack:

  Indirect leak of 2342912 byte(s) in 572 object(s) allocated from:
    #1  ceph::buffer::raw_combined::alloc_data_n_controlblock
        src/common/buffer.cc:114
    #7  MOSDPGLease::encode_payload                src/messages/MOSDPGLease.h:54
    #8  Message::encode                            src/msg/Message.cc:242
    #9  MockMessenger::send_message                src/test/osd/MockMessenger.h:159
    #10 MockPeeringListener::send_cluster_message  src/test/osd/MockPeeringListener.h:193
    #11 PeeringState::send_lease                   src/osd/PeeringState.cc:1305

Three add_ref/consume mismatches conspire to leak the sender Message,
the decoded receiver Message, and its MockConnection:

 - MockPGBackendListener::send_message receives a raw `m` with its +1
   from `new T(...)` and wraps it in `MessageRef(m)` with the default
   add_ref=true, bumping instead of consuming the +1.  Switch to
   add_ref=false.
 - MockPeeringListener::send_cluster_message detaches an intrusive_ptr
   into a raw pointer and then hands it to MockMessenger, which bumps
   with add_ref=true; the +1 transferred via detach() is never
   consumed.  Pass `m.get()` so the caller's intrusive_ptr keeps
   ownership and releases on scope exit.
 - MockMessenger's receiver lambda lets decode_message()'s +1 (the
   Ceph convention) fall out of scope, and constructs the
   `new MockConnection(from_osd)` ConnectionRef with the default
   add_ref=true so the +1 from `new` leaks too.  Construct
   ConnectionRef with add_ref=false, and wrap the decoded Message in a
   `MessageRef{decoded_msg, /*add_ref=*/false}` so the smart pointer
   releases the +1 on every exit path.

While at it, switch the handler API from raw `Message*` / `MsgType*`
to `MessageRef` / `boost::intrusive_ptr<MsgType>` so lifetime is
managed by the smart pointer end-to-end.  Handlers that need to extend
the Message's lifetime call `m.detach()` to transfer the +1 to the
raw pointer the consumer takes ownership of: e.g. OpRequest stores
`Message*` and put()s in its destructor, so PGBackendTestFixture.cc
now passes `m.detach()` into `create_request<OpRequest, Message*>(...)`
in place of an explicit `intrusive_ptr_add_ref(m)` followed by `m`.
The peering handler in ECPeeringTestFixture.cc does not store the
Message; it just lets the typed intrusive_ptr drop on scope exit.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
src/test/osd/ECPeeringTestFixture.cc
src/test/osd/MockMessenger.h
src/test/osd/MockPGBackendListener.h
src/test/osd/MockPeeringListener.h
src/test/osd/PGBackendTestFixture.cc