Skip to content

Commit 9c15517

Browse files
gophergogocaleb2h
authored andcommitted
Cover lifecycle-adapter self+peer deferred-delete pattern (#212)
Adds LifecycleAdapterDefersSelfAndPeerFromOwnCallback, a primitive-level test that models McpServer::ConnectionLifecycleCallbacks: an adapter which, from inside its own onEvent, hands both itself and the owning peer connection to deferredDelete. Confirms neither destructs synchronously inside that callback — doing so would use-after-free the ConnectionCallbacks iterator in production — and that both drain on a later dispatcher pass.
1 parent 444751b commit 9c15517

1 file changed

Lines changed: 81 additions & 0 deletions

File tree

tests/event/test_deferred_delete_ordering.cc

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,87 @@ TEST_F(DeferredDeleteOrderingTest, NestedDeferredDeleteDuringDrainDrainsLater) {
189189
<< "inner queued from outer's destructor must also drain";
190190
}
191191

192+
// Models McpServer::ConnectionLifecycleCallbacks: a callback adapter that, on
193+
// receiving a close-like event, hands *itself* (and a "connection" peer it
194+
// owns) to the dispatcher's deferred-delete queue. Destroying either
195+
// synchronously would be a use-after-free because the adapter's own onEvent()
196+
// is still on the stack, and the connection is still iterating its callback
197+
// list.
198+
struct PeerConnection : public DeferredDeletable {
199+
explicit PeerConnection(std::atomic<bool>& destroyed) : destroyed_(destroyed) {}
200+
~PeerConnection() override { destroyed_ = true; }
201+
std::atomic<bool>& destroyed_;
202+
};
203+
204+
struct LifecycleAdapter : public DeferredDeletable {
205+
LifecycleAdapter(Dispatcher& dispatcher,
206+
std::atomic<bool>& adapter_destroyed,
207+
std::unique_ptr<PeerConnection> peer)
208+
: dispatcher_(dispatcher),
209+
adapter_destroyed_(adapter_destroyed),
210+
peer_(std::move(peer)) {}
211+
~LifecycleAdapter() override { adapter_destroyed_ = true; }
212+
213+
// Mirrors ConnectionLifecycleCallbacks::onEvent: on close, hand both self
214+
// and the peer connection to deferred-delete. Self-move via
215+
// unique_ptr-holder avoids a double-delete.
216+
void onClose(std::unique_ptr<LifecycleAdapter> self_ptr) {
217+
assert(self_ptr.get() == this && "self_ptr must own *this*");
218+
dispatcher_.deferredDelete(std::move(peer_));
219+
dispatcher_.deferredDelete(std::move(self_ptr));
220+
}
221+
222+
Dispatcher& dispatcher_;
223+
std::atomic<bool>& adapter_destroyed_;
224+
std::unique_ptr<PeerConnection> peer_;
225+
};
226+
227+
// Regression test for the crash at McpServer::onConnectionLifecycleEvent:
228+
// when the adapter's own onEvent() hands both itself and the owning
229+
// connection to deferredDelete, neither may run its destructor before the
230+
// onEvent frame returns. If destruction ran synchronously, the caller
231+
// iterating connection callbacks would free and then read this adapter's
232+
// vptr from its own onEvent — immediate UAF.
233+
TEST_F(DeferredDeleteOrderingTest,
234+
LifecycleAdapterDefersSelfAndPeerFromOwnCallback) {
235+
std::atomic<bool> adapter_destroyed{false};
236+
std::atomic<bool> peer_destroyed{false};
237+
std::atomic<bool> observed_alive_after_dispatch{false};
238+
std::atomic<bool> callback_done{false};
239+
240+
auto peer = std::make_unique<PeerConnection>(peer_destroyed);
241+
auto adapter = std::make_unique<LifecycleAdapter>(
242+
*dispatcher_, adapter_destroyed, std::move(peer));
243+
244+
dispatcher_->post([&, adapter_raw = adapter.release()]() mutable {
245+
std::unique_ptr<LifecycleAdapter> self(adapter_raw);
246+
auto* adapter_ptr = self.get();
247+
adapter_ptr->onClose(std::move(self));
248+
// After onClose returns we must still observe both objects alive —
249+
// the dispatcher has queued them but hasn't drained yet.
250+
observed_alive_after_dispatch =
251+
!adapter_destroyed.load() && !peer_destroyed.load();
252+
callback_done = true;
253+
});
254+
255+
for (int i = 0; i < 400 && !callback_done.load(); ++i) {
256+
std::this_thread::sleep_for(5ms);
257+
}
258+
ASSERT_TRUE(callback_done.load());
259+
260+
EXPECT_TRUE(observed_alive_after_dispatch)
261+
<< "adapter or peer was destroyed inside its own onEvent — would UAF "
262+
"the ConnectionCallbacks list iterator in production";
263+
264+
for (int i = 0; i < 400 &&
265+
!(adapter_destroyed.load() && peer_destroyed.load());
266+
++i) {
267+
std::this_thread::sleep_for(5ms);
268+
}
269+
EXPECT_TRUE(adapter_destroyed.load()) << "adapter must eventually drain";
270+
EXPECT_TRUE(peer_destroyed.load()) << "peer must eventually drain";
271+
}
272+
192273
} // namespace
193274
} // namespace event
194275
} // namespace mcp

0 commit comments

Comments
 (0)