Skip to content

Commit 198385b

Browse files
author
Venkatesh Prasad
committed
PS-9719: changing binlog_transaction_dependency_tracking hit segmentation fault
https://perconadev.atlassian.net/browse/PS-9719 Problem ------- When changing binlog_transaction_dependency_tracking in high load workload, MySQL can get a segmentation fault. Analysis -------- Address sanitizer runs exposed the heap-use-after-free. READ of size 8 at 0x6030002c3298 thread T52 #0 _M_hash_code() #1 _M_bucket_index() .. #7 std::unordered_map::insert() #8 Writeset_trx_dependency_tracker::get_dependency() #9 Transaction_dependency_tracker::get_dependency() #10 MYSQL_BIN_LOG::write_transaction() #11 binlog_cache_data::flush() #12 binlog_cache_mngr::flush() #13 MYSQL_BIN_LOG::flush_thread_caches() #14 MYSQL_BIN_LOG::process_flush_stage_queue() #15 MYSQL_BIN_LOG::ordered_commit() #16 MYSQL_BIN_LOG::commit() freed by thread T49 here: #0 operator delete() ... #7 std::unordered_map::clear() #8 Writeset_trx_dependency_tracker::rotate(long) #9 Transaction_dependency_tracker::tracking_mode_changed() #10 update_binlog_transaction_dependency_tracking #11 sys_var::update() - The Writeset_trx_dependency_tracker uses std::unordered_map for storing depdendency information. - When a transaction is committing, the committing thread inserts the dependency information to this map in through get_dependency(). - When the tracking mode is changed, then the map is cleared by Writeset_trx_dependency_tracker::rotate(). Note that no lock/mutex is taken during the rotation. - As the rotate() and get_dependency() operations can be concurrently called from different threads and there is no mutex protection to handle it, it can result in segmentation fault when the get_dependency() tries to insert to the already deleted map. Solution -------- Use std::shared_ptr with atomic load/store for safer dependency tracker map rotation. - Replaced direct usage of of map with std::shared_ptr in the Writeset_trx_dependency_tracker class. - Modified the implementation of rotate() to used std::atomic_load and std::atomic_store to enable thread-safe reads and rotations. With the new solution the rotation happens in an atomic manner. So that transactions calling get_dependency() always use the object returned by shared_ptr. So, even if rotate() happens in parallel, the memory won't be freed until all readers are done.
1 parent 2df90d5 commit 198385b

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

sql/rpl_trx_tracking.cc

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ void Writeset_trx_dependency_tracker::get_dependency(THD *thd,
245245
// it did not broke past the capacity already
246246
!write_set_ctx->was_write_set_limit_reached();
247247
bool exceeds_capacity = false;
248+
auto writeset_history = std::atomic_load(&m_writeset_history);
248249

249250
if (can_use_writesets) {
250251
/*
@@ -253,7 +254,7 @@ void Writeset_trx_dependency_tracker::get_dependency(THD *thd,
253254
using its information for current transaction.
254255
*/
255256
exceeds_capacity =
256-
m_writeset_history.size() + writeset->size() > m_opt_max_history_size;
257+
writeset_history->size() + writeset->size() > m_opt_max_history_size;
257258

258259
/*
259260
Compute the greatest sequence_number among all conflicts and add the
@@ -262,15 +263,15 @@ void Writeset_trx_dependency_tracker::get_dependency(THD *thd,
262263
int64 last_parent = m_writeset_history_start;
263264
for (std::vector<uint64>::iterator it = writeset->begin();
264265
it != writeset->end(); ++it) {
265-
Writeset_history::iterator hst = m_writeset_history.find(*it);
266-
if (hst != m_writeset_history.end()) {
266+
Writeset_history::iterator hst = writeset_history->find(*it);
267+
if (hst != writeset_history->end()) {
267268
if (hst->second > last_parent && hst->second < sequence_number)
268269
last_parent = hst->second;
269270

270271
hst->second = sequence_number;
271272
} else {
272273
if (!exceeds_capacity)
273-
m_writeset_history.insert(
274+
writeset_history->insert(
274275
std::pair<uint64, int64>(*it, sequence_number));
275276
}
276277
}
@@ -292,13 +293,16 @@ void Writeset_trx_dependency_tracker::get_dependency(THD *thd,
292293

293294
if (exceeds_capacity || !can_use_writesets) {
294295
m_writeset_history_start = sequence_number;
295-
m_writeset_history.clear();
296+
if (writeset_history) {
297+
writeset_history->clear();
298+
}
296299
}
297300
}
298301

299302
void Writeset_trx_dependency_tracker::rotate(int64 start) {
300303
m_writeset_history_start = start;
301-
m_writeset_history.clear();
304+
auto new_map = std::make_shared<Writeset_history>();
305+
std::atomic_store(&m_writeset_history, new_map);
302306
}
303307

304308
/**

sql/rpl_trx_tracking.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,10 @@ class Commit_order_trx_dependency_tracker {
127127
class Writeset_trx_dependency_tracker {
128128
public:
129129
Writeset_trx_dependency_tracker(ulong max_history_size)
130-
: m_opt_max_history_size(max_history_size), m_writeset_history_start(0) {}
130+
: m_opt_max_history_size(max_history_size), m_writeset_history_start(0) {
131+
std::atomic_store(&m_writeset_history,
132+
std::make_shared<Writeset_history>());
133+
}
131134

132135
/**
133136
Main function that gets the dependencies using the WRITESET tracker.
@@ -161,7 +164,7 @@ class Writeset_trx_dependency_tracker {
161164
in the database, using row hashes from the writeset as the index.
162165
*/
163166
typedef std::unordered_map<uint64, int64> Writeset_history;
164-
Writeset_history m_writeset_history;
167+
std::shared_ptr<Writeset_history> m_writeset_history;
165168
};
166169

167170
/**

0 commit comments

Comments
 (0)