diff options
| author | Sujatha <sujatha.sivakumar@mariadb.com> | 2019-10-14 12:08:28 +0530 |
|---|---|---|
| committer | Sujatha <sujatha.sivakumar@mariadb.com> | 2019-11-14 12:03:39 +0530 |
| commit | caa79081c31d15d23f790c2cf32eab662a5a5d8e (patch) | |
| tree | b6231cbe41fb9ab7dc2b0a477dc8ea7492203180 /sql/log.cc | |
| parent | 49019dde6515aceb53f38a70bf85d29d18ad9095 (diff) | |
| download | mariadb-git-caa79081c31d15d23f790c2cf32eab662a5a5d8e.tar.gz | |
MDEV-20707: Missing memory barrier in parallel replication error handler in wait_for_prior_commit()
revision-id: 673e253724979fd9fe43a4a22bd7e1b2c3a5269e
Author: Kristian Nielsen
Fix missing memory barrier in wait_for_commit.
The function wait_for_commit::wait_for_prior_commit() has a fast path where it
checks without locks if wakeup_subsequent_commits() has already been called.
This check was missing a memory barrier. The waitee thread does two writes to
variables `waitee' and `wakeup_error', and if the waiting thread sees the
first write it _must_ also see the second or incorrect behavior will occur.
This requires memory barriers between both the writes (release semantics) and
the reads (acquire semantics) of those two variables.
Other accesses to these variables are done under lock or where only one thread
will be accessing them, and can be done without barriers (relaxed semantics).
Diffstat (limited to 'sql/log.cc')
| -rw-r--r-- | sql/log.cc | 19 |
1 files changed, 13 insertions, 6 deletions
diff --git a/sql/log.cc b/sql/log.cc index b90804ed9f0..d7878db6901 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -7481,8 +7481,10 @@ MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *orig_entry) */ wfc= orig_entry->thd->wait_for_commit_ptr; orig_entry->queued_by_other= false; - if (wfc && wfc->waitee) + if (wfc && wfc->waitee.load(std::memory_order_acquire)) { + wait_for_commit *loc_waitee; + mysql_mutex_lock(&wfc->LOCK_wait_commit); /* Do an extra check here, this time safely under lock. @@ -7494,10 +7496,10 @@ MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *orig_entry) before setting the flag, so there is no risk that we can queue ahead of it. */ - if (wfc->waitee && !wfc->waitee->commit_started) + if ((loc_waitee= wfc->waitee.load(std::memory_order_relaxed)) && + !loc_waitee->commit_started) { PSI_stage_info old_stage; - wait_for_commit *loc_waitee; /* By setting wfc->opaque_pointer to our own entry, we mark that we are @@ -7519,7 +7521,8 @@ MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *orig_entry) &wfc->LOCK_wait_commit, &stage_waiting_for_prior_transaction_to_commit, &old_stage); - while ((loc_waitee= wfc->waitee) && !orig_entry->thd->check_killed(1)) + while ((loc_waitee= wfc->waitee.load(std::memory_order_relaxed)) && + !orig_entry->thd->check_killed(1)) mysql_cond_wait(&wfc->COND_wait_commit, &wfc->LOCK_wait_commit); wfc->opaque_pointer= NULL; DBUG_PRINT("info", ("After waiting for prior commit, queued_by_other=%d", @@ -7537,14 +7540,18 @@ MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *orig_entry) do { mysql_cond_wait(&wfc->COND_wait_commit, &wfc->LOCK_wait_commit); - } while (wfc->waitee); + } while (wfc->waitee.load(std::memory_order_relaxed)); } else { /* We were killed, so remove us from the list of waitee. */ wfc->remove_from_list(&loc_waitee->subsequent_commits_list); mysql_mutex_unlock(&loc_waitee->LOCK_wait_commit); - wfc->waitee= NULL; + /* + This is the thread clearing its own status, it is no longer on + the list of waiters. So no memory barriers are needed here. + */ + wfc->waitee.store(NULL, std::memory_order_relaxed); orig_entry->thd->EXIT_COND(&old_stage); /* Interrupted by kill. */ |