lib/wsrep-lib
1
0
Fork 0
mirror of https://github.com/codership/wsrep-lib.git synced 2025-07-03 16:22:35 +03:00
Code Activity

In order to avoid potential deadlocks, release client_state lock when

Browse Source 
calling server state methods which may acquire server_state mutex.

Fixed compilation errors in release mode.
This commit is contained in:
Teemu Ollakka
2018-10-15 10:14:04 +03:00
parent c0c977f9ab
commit 7c6ee3f61f
5 changed files with 28 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
include/wsrep/server_state.hpp
View File

@ -88,6 +88,7 @@
#include "view.hpp" #include "view.hpp"
#include "transaction_id.hpp" #include "transaction_id.hpp"
#include "provider.hpp" #include "provider.hpp"
#include "compiler.hpp"
#include <vector> #include <vector>
#include <string> #include <string>
@ -488,7 +489,8 @@ namespace wsrep
return state(lock); return state(lock);
} }
enum state state(wsrep::unique_lock<wsrep::mutex>& lock) const enum state state(wsrep::unique_lock<wsrep::mutex>&
lock WSREP_UNUSED) const
{ {
assert(lock.owns_lock()); assert(lock.owns_lock());
return state_; return state_;

8
include/wsrep/transaction.hpp
View File

@ -18,8 +18,8 @@
*/ */
/** @file transaction.hpp */ /** @file transaction.hpp */
#ifndef WSREP_TRANSACTION_CONTEXT_HPP #ifndef WSREP_TRANSACTION_HPP
#define WSREP_TRANSACTION_CONTEXT_HPP #define WSREP_TRANSACTION_HPP
#include "provider.hpp" #include "provider.hpp"
#include "server_state.hpp" #include "server_state.hpp"
@ -175,7 +175,7 @@ namespace wsrep
int certify_fragment(wsrep::unique_lock<wsrep::mutex>&); int certify_fragment(wsrep::unique_lock<wsrep::mutex>&);
int certify_commit(wsrep::unique_lock<wsrep::mutex>&); int certify_commit(wsrep::unique_lock<wsrep::mutex>&);
int append_sr_keys_for_commit(); int append_sr_keys_for_commit();
void streaming_rollback(); void streaming_rollback(wsrep::unique_lock<wsrep::mutex>&);
void clear_fragments(); void clear_fragments();
void cleanup(); void cleanup();
void debug_log_state(const char*) const; void debug_log_state(const char*) const;
@ -227,4 +227,4 @@ namespace wsrep
} }
#endif // WSREP_TRANSACTION_CONTEXT_HPP #endif // WSREP_TRANSACTION_HPP

3
src/client_state.cpp
View File

@ -314,13 +314,12 @@ int wsrep::client_state::enter_toi(const wsrep::ws_meta& ws_meta)
int wsrep::client_state::leave_toi() int wsrep::client_state::leave_toi()
{ {
int ret; int ret(0);
if (toi_mode_ == m_local) if (toi_mode_ == m_local)
{ {
switch (provider().leave_toi(id_)) switch (provider().leave_toi(id_))
{ {
case wsrep::provider::success: case wsrep::provider::success:
ret = 0;
break; break;
default: default:
assert(0); assert(0);

3
src/server_state.cpp
View File

@ -929,7 +929,8 @@ int wsrep::server_state::desync(wsrep::unique_lock<wsrep::mutex>& lock)
return ret; return ret;
} }
void wsrep::server_state::resync(wsrep::unique_lock<wsrep::mutex>& lock) void wsrep::server_state::resync(wsrep::unique_lock<wsrep::mutex>&
lock WSREP_UNUSED)
{ {
assert(lock.owns_lock()); assert(lock.owns_lock());
assert(desync_count_ > 0); assert(desync_count_ > 0);

27
src/transaction.cpp
View File

@ -520,7 +520,9 @@ int wsrep::transaction::after_commit()
client_state_.mode() == wsrep::client_state::m_high_priority); client_state_.mode() == wsrep::client_state::m_high_priority);
if (client_state_.mode() == wsrep::client_state::m_local) if (client_state_.mode() == wsrep::client_state::m_local)
{ {
lock.unlock();
client_state_.server_state_.stop_streaming_client(&client_state_); client_state_.server_state_.stop_streaming_client(&client_state_);
lock.lock();
} }
clear_fragments(); clear_fragments();
} }
@ -573,7 +575,7 @@ int wsrep::transaction::before_rollback()
// Voluntary rollback // Voluntary rollback
if (is_streaming()) if (is_streaming())
{ {
streaming_rollback(); streaming_rollback(lock);
} }
state(lock, s_aborting); state(lock, s_aborting);
break; break;
@ -586,7 +588,7 @@ int wsrep::transaction::before_rollback()
{ {
if (is_streaming()) if (is_streaming())
{ {
streaming_rollback(); streaming_rollback(lock);
} }
state(lock, s_aborting); state(lock, s_aborting);
} }
@ -594,14 +596,14 @@ int wsrep::transaction::before_rollback()
case s_cert_failed: case s_cert_failed:
if (is_streaming()) if (is_streaming())
{ {
streaming_rollback(); streaming_rollback(lock);
} }
state(lock, s_aborting); state(lock, s_aborting);
break; break;
case s_aborting: case s_aborting:
if (is_streaming()) if (is_streaming())
{ {
streaming_rollback(); streaming_rollback(lock);
} }
break; break;
case s_must_replay: case s_must_replay:
@ -899,7 +901,7 @@ bool wsrep::transaction::bf_abort(
if (client_state_.mode() == wsrep::client_state::m_local && if (client_state_.mode() == wsrep::client_state::m_local &&
is_streaming() && state() == s_executing) is_streaming() && state() == s_executing)
{ {
streaming_rollback(); streaming_rollback(lock);
} }
if ((client_state_.state() == wsrep::client_state::s_idle && if ((client_state_.state() == wsrep::client_state::s_idle &&
@ -918,8 +920,10 @@ bool wsrep::transaction::bf_abort(
state(lock, wsrep::transaction::s_aborting); state(lock, wsrep::transaction::s_aborting);
if (client_state_.mode() == wsrep::client_state::m_high_priority) if (client_state_.mode() == wsrep::client_state::m_high_priority)
{ {
lock.unlock();
client_state_.server_state().stop_streaming_applier( client_state_.server_state().stop_streaming_applier(
server_id_, id_); server_id_, id_);
lock.lock();
} }
lock.unlock(); lock.unlock();
@ -1170,11 +1174,13 @@ int wsrep::transaction::certify_fragment(
{ {
if (is_streaming() == false) if (is_streaming() == false)
{ {
lock.unlock();
client_state_.server_state_.stop_streaming_client(&client_state_); client_state_.server_state_.stop_streaming_client(&client_state_);
lock.lock();
} }
else else
{ {
streaming_rollback(); streaming_rollback(lock);
} }
if (state_ != s_must_abort) if (state_ != s_must_abort)
{ {
@ -1186,7 +1192,7 @@ int wsrep::transaction::certify_fragment(
{ {
if (is_streaming()) if (is_streaming())
{ {
streaming_rollback(); streaming_rollback(lock);
} }
client_state_.override_error(wsrep::e_deadlock_error, cert_ret); client_state_.override_error(wsrep::e_deadlock_error, cert_ret);
ret = 1; ret = 1;
@ -1389,7 +1395,7 @@ int wsrep::transaction::append_sr_keys_for_commit()
return ret; return ret;
} }
void wsrep::transaction::streaming_rollback() void wsrep::transaction::streaming_rollback(wsrep::unique_lock<wsrep::mutex>& lock)
{ {
debug_log_state("streaming_rollback enter"); debug_log_state("streaming_rollback enter");
assert(state_ != s_must_replay); assert(state_ != s_must_replay);
@ -1399,7 +1405,9 @@ void wsrep::transaction::streaming_rollback()
{ {
if (bf_aborted_in_total_order_) if (bf_aborted_in_total_order_)
{ {
lock.unlock();
client_state_.server_state_.stop_streaming_client(&client_state_); client_state_.server_state_.stop_streaming_client(&client_state_);
lock.lock();
streaming_context_.rolled_back(id_); streaming_context_.rolled_back(id_);
} }
else else
@ -1409,9 +1417,10 @@ void wsrep::transaction::streaming_rollback()
// Adopt transaction will copy fragment set and appropriate // Adopt transaction will copy fragment set and appropriate
// meta data. Mark current transaction streaming context // meta data. Mark current transaction streaming context
// rolled back. // rolled back.
lock.unlock();
client_state_.server_state_.convert_streaming_client_to_applier( client_state_.server_state_.convert_streaming_client_to_applier(
&client_state_); &client_state_);
lock.lock();
streaming_context_.cleanup(); streaming_context_.cleanup();
streaming_context_.rolled_back(id_); streaming_context_.rolled_back(id_);
enum wsrep::provider::status ret; enum wsrep::provider::status ret;