Comment 4 for bug 1309026

Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

There is a commit which intended to fix similar issue for debug builds.

        revno: 0.18338.68
        committer: Andrei Elkin <email address hidden>
        branch nick: 5.6-fixes
        timestamp: Wed 2012-11-21 09:45:32 +0200
        message:
          Bug#15867985 STRESS TESTS CAUSES DBUG SERVER TO EXIT WITH SIGNAL 6

          When run with --mysqld=--binlog-order-commits=1 (default) mysqld executes a
          piece of BGC logics that tames a group of threads in a way that they give up
          their commit execution path to a leader thread.
          This preemption appeared to have a specific to DBUG_ON build flaw in that
          the leader and a follower thread could modify a thread-specific area
          concurrently which led to a segfault.

          Fixed with refining the preemption logics to be thread-safe for the DBUG_ON build.

bzr diff -c0.18338.68 sergei/bzr/issue40387/483
=== modified file 'sql/binlog.cc'
--- sql/binlog.cc 2012-11-15 03:35:25 +0000
+++ sql/binlog.cc 2012-11-21 07:45:32 +0000
@@ -1334,6 +1334,16 @@
   if (!leader)
   {
     mysql_mutex_lock(&m_lock_done);
+#ifndef DBUG_OFF
+ /*
+ Leader can be awaiting all-clear to preempt follower's execution.
+ With setting the status the follower ensures it won't execute anything
+ including thread-specific code.
+ */
+ thd->transaction.flags.ready_preempt= 1;
+ if (leader_await_preempt_status)
+ mysql_cond_signal(&m_cond_preempt);
+#endif
     while (thd->transaction.flags.pending)
       mysql_cond_wait(&m_cond_done, &m_lock_done);
     mysql_mutex_unlock(&m_lock_done);
@@ -1361,6 +1371,22 @@
   DBUG_RETURN(result);
 }

+#ifndef DBUG_OFF
+void Stage_manager::clear_preempt_status(THD *head)
+{
+ DBUG_ASSERT(head);
+
+ mysql_mutex_lock(&m_lock_done);
+ while(!head->transaction.flags.ready_preempt)
+ {
+ leader_await_preempt_status= true;
+ mysql_cond_wait(&m_cond_preempt, &m_lock_done);
+ }
+ leader_await_preempt_status= false;
+ mysql_mutex_unlock(&m_lock_done);
+}
+#endif
+
 /**
   Write a rollback record of the transaction to the binary log.

@@ -6009,6 +6035,9 @@
 {
   mysql_mutex_assert_owner(&LOCK_commit);
   Thread_excursion excursion(thd);
+#ifndef DBUG_OFF
+ thd->transaction.flags.ready_preempt= 1; // formality by the leader
+#endif
   for (THD *head= first ; head ; head = head->next_to_commit)
   {
     DBUG_PRINT("debug", ("Thread ID: %lu, commit_error: %d, flags.pending: %s",
@@ -6022,6 +6051,9 @@
       If flush succeeded, attach to the session and commit it in the
       engines.
     */
+#ifndef DBUG_OFF
+ stage_manager.clear_preempt_status(head);
+#endif
     if (flush_error != 0)
       head->commit_error= flush_error;
     else if (int error= excursion.attach_to(head))
@@ -6031,6 +6063,9 @@
       bool all= head->transaction.flags.real_commit;
       if (head->transaction.flags.commit_low)
       {
+ /* head is parked to have exited append() */
+ DBUG_ASSERT(head->transaction.flags.ready_preempt);
+
         if (int error= ha_commit_low(head, all))
           head->commit_error= error;
         else if (head->transaction.flags.xid_written)
@@ -6269,6 +6304,17 @@
   thd->transaction.flags.real_commit= all;
   thd->transaction.flags.xid_written= false;
   thd->transaction.flags.commit_low= !skip_commit;
+#ifndef DBUG_OFF
+ /*
+ The group commit Leader may have to wait for follower whose transaction
+ is not ready to be preempted. Initially the status is pessimistic.
+ Preemption guarding logics is necessary only when DBUG_ON is set.
+ It won't be required for the dbug-off case as long as the follower won't
+ execute any thread-specific write access code in this method, which is
+ the case as of current.
+ */
+ thd->transaction.flags.ready_preempt= 0;
+#endif

   DBUG_PRINT("enter", ("flags.pending: %s, commit_error: %d, thread_id: %lu",
                        YESNO(thd->transaction.flags.pending),

=== modified file 'sql/binlog.h'
--- sql/binlog.h 2012-09-18 12:32:43 +0000
+++ sql/binlog.h 2012-11-21 07:45:32 +0000
@@ -119,6 +119,10 @@
   {
     mysql_mutex_init(key_LOCK_done, &m_lock_done, MY_MUTEX_INIT_FAST);
     mysql_cond_init(key_COND_done, &m_cond_done, NULL);
+#ifndef DBUG_OFF
+ /* reuse key_COND_done 'cos a new PSI object would be wasteful in DBUG_ON */
+ mysql_cond_init(key_COND_done, &m_cond_preempt, NULL);
+#endif
     m_queue[FLUSH_STAGE].init(
 #ifdef HAVE_PSI_INTERFACE
                               key_LOCK_flush_queue
@@ -155,6 +159,7 @@
     If wait_if_follower is true the thread is not the stage leader,
     the thread will be wait for the queue to be processed by the
     leader before it returns.
+ In DBUG-ON version the follower marks is preempt status as ready.

     @param stage Stage identifier for the queue to append to.
     @param first Queue to append.
@@ -172,6 +177,17 @@
     return m_queue[stage].pop_front();
   }

+#ifndef DBUG_OFF
+ /**
+ The method ensures the follower's execution path can be preempted
+ by the leader's thread.
+ Preempt status of @c head follower is checked to engange the leader
+ into waiting when set.
+
+ @param head THD* of a follower thread
+ */
+ void clear_preempt_status(THD *head);
+#endif

   /**
     Fetch the entire queue and empty it.
@@ -206,6 +222,13 @@

   /** Mutex used for the condition variable above */
   mysql_mutex_t m_lock_done;
+#ifndef DBUG_OFF
+ /** Flag is set by Leader when it starts waiting for follower's all-clear */
+ bool leader_await_preempt_status;
+
+ /** Condition variable to indicate a follower started waiting for commit */
+ mysql_cond_t m_cond_preempt;
+#endif
 };

=== modified file 'sql/sql_class.h'
--- sql/sql_class.h 2012-11-20 13:08:53 +0000
+++ sql/sql_class.h 2012-11-21 07:45:32 +0000
@@ -2352,6 +2352,9 @@
       bool xid_written:1; // The session wrote an XID
       bool real_commit:1; // Is this a "real" commit?
       bool commit_low:1; // see MYSQL_BIN_LOG::ordered_commit
+#ifndef DBUG_OFF
+ bool ready_preempt:1; // internal in MYSQL_BIN_LOG::ordered_commit
+#endif
     } flags;

     void cleanup()

With this logic enabled there is no crash.