lib/wsrep-lib
1
0
Fork 0
mirror of https://github.com/codership/wsrep-lib.git synced 2026-10-23 09:48:57 +03:00
Code Activity

MENT-2552 Fix mutex order in commit_or_rollback_by_xid()

Browse Source 
The server state mutex must always be acquired before the client state
mutex. server_state::convert_streaming_client_to_applier() holds the
server state mutex over high_priority_service::adopt_transaction(),
after_apply(), store_globals() and
server_service::release_high_priority_service(), all of which take the
client state mutex of the applier client.
server_state::close_orphaned_sr_transactions() does the same.

transaction::commit_or_rollback_by_xid(), reached by XA COMMIT or
XA ROLLBACK of an xid owned by a streaming applier, took the two in the
opposite order: it locked the client state and then called
server_state::find_streaming_applier(), which locks the server state.
A client terminating a prepared XA transaction by xid could therefore
wait for the server state mutex while holding its own client state
mutex, deadlocking against a thread which is converting a streaming
client into a streaming applier.

Do the find_streaming_applier() lookup before locking the client state.
Nothing between the old lock point and the lookup needs the client
state mutex.

Add sync point wsrep_commit_or_rollback_by_xid_client_locked so that
the mutex order can be exercised from a test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jan Lindström
2026-09-01 14:52:35 +03:00
parent cf331a00c7
commit 66a2add655
1 changed files with 14 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/transaction.cpp
View File

@@ -1176,10 +1176,23 @@ int wsrep::transaction::commit_or_rollback_by_xid(const wsrep::xid& xid,
bool commit)
{
debug_log_state("commit_or_rollback_by_xid enter");
wsrep::unique_lock<wsrep::mutex> lock(client_state_.mutex_);
wsrep::server_state& server_state(client_state_.server_state());
/* Look up the streaming applier which owns the xid before locking the
client state. find_streaming_applier() grabs the server state mutex,
and the server state mutex must always be acquired before the client
state mutex, never the other way around. The server state is locked
first for example in convert_streaming_client_to_applier(), which
calls into high_priority_service::adopt_transaction(),
after_apply(), store_globals() and
server_service::release_high_priority_service(), all of which take
the client state mutex of the applier client. Taking the two mutexes
in the opposite order here would deadlock against those. */
wsrep::high_priority_service* sa(server_state.find_streaming_applier(xid));
wsrep::unique_lock<wsrep::mutex> lock(client_state_.mutex_);
client_service_.debug_sync("wsrep_commit_or_rollback_by_xid_client_locked");
if (!sa)
{
assert(sa);