BUG#39053 - UNISTALL PLUGIN does not allow the storage engine

to cleanup open connections

It was possible to UNINSTALL storage engine plugin when binding
between THD object and storage engine is still active (e.g. in
the middle of transaction).

To avoid unclean deactivation (uninstall) of storage engine plugin
in the middle of transaction, additional storage engine plugin
lock is acquired by thd_set_ha_data().

If ha_data is not null and storage engine plugin was not locked
by thd_set_ha_data() in this connection before, storage engine
plugin gets locked.

If ha_data is null and storage engine plugin was locked by
thd_set_ha_data() in this connection before, storage engine
plugin lock gets released.

If handlerton::close_connection() didn't reset ha_data, server does
it immediately after calling handlerton::close_connection().

Note that this is just a framework fix, storage engines must switch
to thd_set_ha_data() from thd_ha_data() if they want to see fit.

include/mysql/plugin.h:
  As thd_{get|set}_ha_data() have some extra logic now, they
  must be implemented on server side.
include/mysql/plugin.h.pp:
  As thd_{get|set}_ha_data() have some extra logic now, they
  must be implemented on server side.
sql/handler.cc:
  Make sure ha_data is reset and ha_data lock is released.
sql/handler.h:
  hton is not supposed to be updated by ha_lock_engine(),
  make it const.
sql/sql_class.cc:
  As thd_{get|set}_ha_data() have some extra logic now, they
  must be implemented on server side.
sql/sql_class.h:
  Added ha_data lock.

include/mysql/plugin.h

@@ -801,30 +801,37 @@ void mysql_query_cache_invalidate4(MYSQL_THD thd,
                                    const char *key, unsigned int key_length,
                                    int using_trx);
 
-#ifdef __cplusplus
-}
-#endif
 
-#ifdef __cplusplus
 /**
   Provide a handler data getter to simplify coding
 */
-inline
+void *thd_get_ha_data(const MYSQL_THD thd, const struct handlerton *hton);
-void *
+
-thd_get_ha_data(const MYSQL_THD thd, const struct handlerton *hton)
-{
-  return *thd_ha_data(thd, hton);
-}
 
 /**
   Provide a handler data setter to simplify coding
+
+  @details
+  Set ha_data pointer (storage engine per-connection information).
+
+  To avoid unclean deactivation (uninstall) of storage engine plugin
+  in the middle of transaction, additional storage engine plugin
+  lock is acquired.
+
+  If ha_data is not null and storage engine plugin was not locked
+  by thd_set_ha_data() in this connection before, storage engine
+  plugin gets locked.
+
+  If ha_data is null and storage engine plugin was locked by
+  thd_set_ha_data() in this connection before, storage engine
+  plugin lock gets released.
+
+  If handlerton::close_connection() didn't reset ha_data, server does
+  it immediately after calling handlerton::close_connection().
 */
-inline
+void thd_set_ha_data(MYSQL_THD thd, const struct handlerton *hton,
-void
+                     const void *ha_data);
-thd_set_ha_data(const MYSQL_THD thd, const struct handlerton *hton,
+#ifdef __cplusplus
-                const void *ha_data)
-{
-  *thd_ha_data(thd, hton)= (void*) ha_data;
 }
 #endif
 

include/mysql/plugin.h.pp

@@ -137,3 +137,6 @@ void thd_get_xid(const void* thd, MYSQL_XID *xid);
 void mysql_query_cache_invalidate4(void* thd,
                                    const char *key, unsigned int key_length,
                                    int using_trx);
+void *thd_get_ha_data(const void* thd, const struct handlerton *hton);
+void thd_set_ha_data(void* thd, const struct handlerton *hton,
+                     const void *ha_data);

sql/handler.cc

@@ -159,7 +159,7 @@ redo:
 }
 
 
-plugin_ref ha_lock_engine(THD *thd, handlerton *hton)
+plugin_ref ha_lock_engine(THD *thd, const handlerton *hton)
 {
   if (hton)
   {
@@ -601,9 +601,13 @@ static my_bool closecon_handlerton(THD *thd, plugin_ref plugin,
     there's no need to rollback here as all transactions must
     be rolled back already
   */
-  if (hton->state == SHOW_OPTION_YES && hton->close_connection &&
+  if (hton->state == SHOW_OPTION_YES && thd_get_ha_data(thd, hton))
-      thd_get_ha_data(thd, hton))
+  {
+    if (hton->close_connection)
       hton->close_connection(hton, thd);
+    /* make sure ha_data is reset and ha_data_lock is released */
+    thd_set_ha_data(thd, hton, NULL);
+  }
   return FALSE;
 }
 

sql/handler.h

@@ -1956,7 +1956,7 @@ extern ulong total_ha, total_ha_2pc;
 /* lookups */
 handlerton *ha_default_handlerton(THD *thd);
 plugin_ref ha_resolve_by_name(THD *thd, const LEX_STRING *name);
-plugin_ref ha_lock_engine(THD *thd, handlerton *hton);
+plugin_ref ha_lock_engine(THD *thd, const handlerton *hton);
 handlerton *ha_resolve_by_legacy_type(THD *thd, enum legacy_db_type db_type);
 handler *get_new_handler(TABLE_SHARE *share, MEM_ROOT *alloc,
                          handlerton *db_type);

sql/sql_class.cc

@@ -284,6 +284,37 @@ void **thd_ha_data(const THD *thd, const struct handlerton *hton)
   return (void **) &thd->ha_data[hton->slot].ha_ptr;
 }
 
+
+/**
+  Provide a handler data getter to simplify coding
+*/
+extern "C"
+void *thd_get_ha_data(const THD *thd, const struct handlerton *hton)
+{
+  return *thd_ha_data(thd, hton);
+}
+
+
+/**
+  Provide a handler data setter to simplify coding
+  @see thd_set_ha_data() definition in plugin.h
+*/
+extern "C"
+void thd_set_ha_data(THD *thd, const struct handlerton *hton,
+                     const void *ha_data)
+{
+  plugin_ref *lock= &thd->ha_data[hton->slot].lock;
+  if (ha_data && !*lock)
+    *lock= ha_lock_engine(NULL, (handlerton*) hton);
+  else if (!ha_data && *lock)
+  {
+    plugin_unlock(NULL, *lock);
+    *lock= NULL;
+  }
+  *thd_ha_data(thd, hton)= (void*) ha_data;
+}
+
+
 extern "C"
 long long thd_test_options(const THD *thd, long long test_options)
 {

sql/sql_class.h

@@ -1264,7 +1264,11 @@ struct Ha_data
     @sa trans_register_ha()
   */
   Ha_trx_info ha_info[2];
-
+  /**
+    NULL: engine is not bound to this thread
+    non-NULL: engine is bound to this thread, engine shutdown forbidden
+  */
+  plugin_ref lock;
   Ha_data() :ha_ptr(NULL) {}
 };