Skip to content

Commit e0f1cd7

Browse files
committed
type-context.h: Extent cancel_mutex lock to prevent theoretical race
As pointed out by janb84 in bitcoin/bitcoin#34422 (comment) it makes sense for the on_cancel callback to lock cancel_mutex while it is assigning request_canceled = true. The lock and assigment were introduced in bitcoin-core#240 and in an earlier version of that PR, request_canceled was a std::atomic and the assignment happened before the lock was acquired instead of after, so it was ok for the lock to be unnamed and immediately released after being acquired. But in the final verion of bitcoin-core#240 request_canceled is an ordinary non-atomic bool, and it should be assigned true with the lock held to prevent a theoretical race condition where capn'proto event loop cancels the request before the execution thread runs, and the execution thread sees the old request_canceled = false value and then unsafely accesses deleted parameters. The request being canceled so quickly and parameters being accessed so slowly, and stale request_canceled value being read even after the execution thread has the cancel_mutex lock should be very unlikely to occur in practice, but could happen in theory and is good to fix.
1 parent 290702c commit e0f1cd7

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

include/mp/type-context.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
123123
// it. So in addition to locking the mutex, the
124124
// execution thread always checks request_canceled
125125
// as well before accessing the structs.
126-
Lock{cancel_mutex};
126+
Lock cancel_lock{cancel_mutex};
127127
server_context.request_canceled = true;
128128
};
129129
// Update requests_threads map if not canceled.

0 commit comments

Comments
 (0)