Bug #93362 code bug
Submitted: 27 Nov 2018 13:09 Modified: 29 Nov 2018 23:19
Reporter: baogang hou Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S2 (Serious)
Version:8.0.13 OS:CentOS
Assigned to: CPU Architecture:Other

[27 Nov 2018 13:09] baogang hou
Description:
I think there is a problem with the following code, which will cause the system to hang up.if acquire_uncached_uncommitted return false ,the index_space is null,the server will down.

dict0dd.cc:3977 

{
      if (client->acquire_uncached_uncommitted<dd::Tablespace>(index_space_id,
                                                               &index_space)) {
        my_error(ER_TABLESPACE_MISSING, MYF(0), m_table->name.m_name);
        fail = true;
        break;
      }

      if (index_space->se_private_data().get_uint32(
              dd_space_key_strings[DD_SPACE_ID], &sid)) {
        fail = true;
        break;
      }
    }
dictionary_client.cc:1145
template <typename T>
bool Dictionary_client::acquire_uncached_uncommitted(Object_id id, T **object) {
  const typename T::Id_key key(id);
  DBUG_ASSERT(object);

  // First get the object from acquire_uncommitted. This should be safe
  // even without MDL, since the object is only available to this thread.
  typename T::Cache_partition *uncommitted_object = nullptr;
  bool dropped = false;
  acquire_uncommitted(key, &uncommitted_object, &dropped);

  // In this case, if the object has been dropped, we return nullptr since
  // this is in line with the isolation level for the disk access.
  if (dropped) {
    *object = nullptr;
    return false;
  }

  if (uncommitted_object != nullptr) {
    // Dynamic cast may legitimately return NULL if we e.g. asked
    // for a dd::Table and got a dd::View in return, but in this
    // case, we cannot delete the stored_object since it is present
    // in the uncommitted registry. The returned object, however,
    // must be auto deleted.
    *object =
        const_cast<T *>(dynamic_cast<const T *>(uncommitted_object->clone()));
    if (*object != nullptr) auto_delete<T>(*object);
    return false;
  }

  // Read the uncached dictionary object using ISO_READ_UNCOMMITTED
  // isolation level.
  const typename T::Cache_partition *stored_object = nullptr;
  bool error = Shared_dictionary_cache::instance()->get_uncached(
      m_thd, key, ISO_READ_UNCOMMITTED, &stored_object);
  if (!error) {
    // Here, stored_object is a newly created instance, so we do not need to
    // clone() it, but we must delete it if dynamic cast fails.
    *object = const_cast<T *>(dynamic_cast<const T *>(stored_object));
    if (stored_object && !*object)
      delete stored_object;
    else
      auto_delete<T>(*object);
  } else
    DBUG_ASSERT(m_thd->is_error() || m_thd->killed);

  return error;
}

if acquire_uncached_uncommitted return false ,the index_space is null

the server will down.

How to repeat:
There is something wrong with the code logic, so you don't need to reproduce it. Look at the code logic, you can see it.

Suggested fix:
if( client->acquire_uncached_uncommitted<dd::Tablespace>(
                        index_space_id, &index_space) ||
                    (*index_space) == nullptr ||
                    (*index_space)->se_private_data().get_uint32(
              dd_space_key_strings[DD_SPACE_ID], &sid)))
{
        fail = true;
        break;
}
[27 Nov 2018 14:16] MySQL Verification Team
Hi,

Thank you for your bug report.

I find that your analysis is  correct.

Verified as reported.
[28 Nov 2018 3:33] baogang hou
Yesterday accidentally pasted the wrong fixed code, you can refer to the following code, it is correct.
if(client->acquire_uncached_uncommitted<dd::Tablespace>(
                        index_space_id, &index_space) ||
                     index_space == nullptr ||
                     index_space->se_private_data().get_uint32(
              dd_space_key_strings[DD_SPACE_ID], &sid))
     {
        fail = true;
        break;
     }
[29 Nov 2018 23:19] Daniel Price
Posted by developer:
 
Fixed as of the upcoming 8.0.15  release, and here's the changelog entry:

Data dictionary code did not check for a returned data dictionary object,
causing a hang condition.
[4 Dec 2018 12:55] Daniel Price
Posted by developer:
 
Revised changelog entry:

"Data dictionary code did not check for a returned data dictionary object,
which could potentially cause the server to exit due to a null pointer
access."