Skip to content

Commit 415081a

Browse files
committed
race fix: m_on_cancel called after request finishes
This fixes a race condition in the mp.Context PassField() overload which is used to execute async requests, that can currently trigger segfaults as reported in bitcoin/bitcoin#34782 when a cancellation happens after the request executes but before it returns. The bug can be reproduced by running the unit test added in the previous commit and was also seen in antithesis (see details in linked issue), but should be unlikely to happen normally because the cancellation would have to happen in a very short window for there to be a problem. This bug was introduced commit in 0174450 which started to cancel requests on disconnects. Before that commit a cancellation callback was not present. This fix was originally posted in bitcoin/bitcoin#34782 (comment) and there is a sequence diagram explaining the bug in bitcoin-core#249 (comment)
1 parent 13870f9 commit 415081a

1 file changed

Lines changed: 9 additions & 0 deletions

File tree

include/mp/type-context.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,15 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
157157
// the disconnect handler trying to destroy the thread
158158
// client object.
159159
server.m_context.loop->sync([&] {
160+
// Clear cancellation callback. At this point the
161+
// method invocation finished and the result is
162+
// either being returned, or discarded if a
163+
// cancellation happened. So we do not need to be
164+
// notified of cancellations after this point. Also
165+
// we do not want to be notified because
166+
// cancel_mutex and server_context could be out of
167+
// scope when it happens.
168+
cancel_monitor.m_on_cancel = nullptr;
160169
auto self_dispose{kj::mv(self)};
161170
if (erase_thread) {
162171
// Look up the thread again without using existing

0 commit comments

Comments
 (0)