MDEV-16401: Apply review comments to MDEV-16005

Do not hold LOCK_thd_data during my_error or WSREP_DEBUG. Similarly, release LOCK_thd_data before close_thread_tables() call.

Author: janlindstrom

sql/sql_parse.cc

@@ -1271,9 +1271,9 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
  if (thd->wsrep_conflict_state == ABORTED &&
  command != COM_STMT_CLOSE && command != COM_QUIT)
  {
+ mysql_mutex_unlock(&thd->LOCK_thd_data);
  my_error(ER_LOCK_DEADLOCK, MYF(0), "wsrep aborted transaction");
  WSREP_DEBUG("Deadlock error for: %s", thd->query());
- mysql_mutex_unlock(&thd->LOCK_thd_data);
  thd->reset_killed();
  thd->mysys_var->abort = 0;
  thd->wsrep_conflict_state = NO_CONFLICT;
@@ -7290,34 +7290,44 @@ static void wsrep_mysql_parse(THD *thd, char *rawbuf, uint length,
  thd->lex->sql_command != SQLCOM_SELECT &&
  (thd->wsrep_retry_counter < thd->variables.wsrep_retry_autocommit))
  {
- WSREP_DEBUG("wsrep retrying AC query: %s", 
+ mysql_mutex_unlock(&thd->LOCK_thd_data);
+ WSREP_DEBUG("wsrep retrying AC query: %s",
  (thd->query()) ? thd->query() : "void");
 
  /* Performance Schema Interface instrumentation, end */
  MYSQL_END_STATEMENT(thd->m_statement_psi, thd->get_stmt_da());
  thd->m_statement_psi= NULL;
  thd->m_digest= NULL;
+ // Released thd->LOCK_thd_data above as below could end up
+ // close_thread_tables()/close_open_tables()/close_thread_table()/mysql_mutex_lock(&thd->LOCK_thd_data)
  close_thread_tables(thd);
 
+ mysql_mutex_lock(&thd->LOCK_thd_data);
  thd->wsrep_conflict_state= RETRY_AUTOCOMMIT;
  thd->wsrep_retry_counter++; // grow
  wsrep_copy_query(thd);
  thd->set_time();
  parser_state->reset(rawbuf, length);
+ mysql_mutex_unlock(&thd->LOCK_thd_data);
  }
  else
  {
- WSREP_DEBUG("%s, thd: %lu is_AC: %d, retry: %lu - %lu SQL: %s", 
- (thd->wsrep_conflict_state == ABORTED) ? 
+ mysql_mutex_unlock(&thd->LOCK_thd_data);
+ // This does dirty read to wsrep variables but it is only a debug code
+ WSREP_DEBUG("%s, thd: %lu is_AC: %d, retry: %lu - %lu SQL: %s",
+ (thd->wsrep_conflict_state == ABORTED) ?
  "BF Aborted" : "cert failure",
- thd->thread_id, is_autocommit, thd->wsrep_retry_counter, 
+ thd->thread_id, is_autocommit, thd->wsrep_retry_counter,
  thd->variables.wsrep_retry_autocommit, thd->query());
  my_error(ER_LOCK_DEADLOCK, MYF(0), "wsrep aborted transaction");
+
+ mysql_mutex_lock(&thd->LOCK_thd_data);
  thd->wsrep_conflict_state= NO_CONFLICT;
  if (thd->wsrep_conflict_state != REPLAYING)
  thd->wsrep_retry_counter= 0; // reset
+ mysql_mutex_unlock(&thd->LOCK_thd_data);
  }
- mysql_mutex_unlock(&thd->LOCK_thd_data);
+
  thd->reset_killed();
  }
  else
@@ -7353,6 +7363,7 @@ static void wsrep_mysql_parse(THD *thd, char *rawbuf, uint length,
 #endif /* WITH_WSREP */
 }
 
+
 /*
  When you modify mysql_parse(), you may need to modify
  mysql_test_parse_for_slave() in this same file.