Bug #96552 Replication filter iterator needs to be re-assigned after erase() in looping
Submitted: 15 Aug 12:55 Modified: 21 Aug 13:07
Reporter: Xing Ai (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: Replication Severity:S5 (Performance)
Version:8.0 OS:Any
Assigned to: CPU Architecture:Any
Tags: multisource replication, SQL

[15 Aug 12:55] Xing Ai
Description:
This problem is found through code review instead of testing.

The source file "https://github.com/mysql/mysql-server/blob/8.0/sql/rpl_msr.cc" has iterator erase problem in looping. This
 may cause undefined behavior or stability problem.

The iterator becomes invalid after its element is removed. "it++" would cause undefined behavior due to such invalidation.

How to repeat:
In the code snippet below, the line of "channel_to_filter.erase(it);" has problem.

void Rpl_channel_filters::delete_filter(Rpl_filter *rpl_filter) {
  DBUG_ENTER("Rpl_channel_filters::delete_filter");

  /* Traverse the filter map. */
  m_channel_to_filter_lock->wrlock();
  for (filter_map::iterator it = channel_to_filter.begin();
       it != channel_to_filter.end(); it++) {
    if (it->second == rpl_filter) {
      /* Find the replication filter and delete it. */
      delete it->second;
      channel_to_filter.erase(it);
#ifdef WITH_PERFSCHEMA_STORAGE_ENGINE
      reset_pfs_view();
#endif /* WITH_PERFSCHEMA_STORAGE_ENGINE */
      m_channel_to_filter_lock->unlock();
      DBUG_VOID_RETURN;
    }
  }
  m_channel_to_filter_lock->unlock();

  DBUG_VOID_RETURN;
}

Suggested fix:
The fix of one line change is very subtle as below.

void Rpl_channel_filters::delete_filter(Rpl_filter *rpl_filter) {
  DBUG_ENTER("Rpl_channel_filters::delete_filter");

  /* Traverse the filter map. */
  m_channel_to_filter_lock->wrlock();
  for (filter_map::iterator it = channel_to_filter.begin();
       it != channel_to_filter.end(); it++) {
    if (it->second == rpl_filter) {
      /* Find the replication filter and delete it. */
      delete it->second;
      it = channel_to_filter.erase(it);
#ifdef WITH_PERFSCHEMA_STORAGE_ENGINE
      reset_pfs_view();
#endif /* WITH_PERFSCHEMA_STORAGE_ENGINE */
      m_channel_to_filter_lock->unlock();
      DBUG_VOID_RETURN;
    }
  }
  m_channel_to_filter_lock->unlock();

  DBUG_VOID_RETURN;
}
[19 Aug 10:49] Bogdan Kecman
Hi,

Seems you are right. Thanks for submitting the report.

In order to submit contributions you must first sign the Oracle Contribution Agreement (OCA).
For additional information please check http://www.oracle.com/technetwork/community/oca-486395.html.
If you have any questions, please contact the MySQL community team.
[21 Aug 13:07] Xing Ai
Thanks for looking into the bug. I'll sign the OCA.