Skip to content

Commit b976fb1

Browse files
author
Konstantin Osipov
committed
A review comment for WL#4441 " LOCK_open: Remove requirement of
mutex protecting thd->open_tables". We should not manipulate with table->s->version outside the table definition cache code, but use the TDC API to achieve the desired result. Fix one violation: close_all_tables_for_name().
1 parent 0cb90ed commit b976fb1

File tree

3 files changed

+48
-6
lines changed

3 files changed

+48
-6
lines changed

sql/sql_base.cc

+18-6
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,9 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
10221022
TABLE_LIST *tables_to_reopen= (tables ? tables :
10231023
thd->locked_tables_list.locked_tables());
10241024

1025+
/* Close open HANLER instances to avoid self-deadlock. */
1026+
mysql_ha_flush_tables(thd, tables_to_reopen);
1027+
10251028
for (TABLE_LIST *table_list= tables_to_reopen; table_list;
10261029
table_list= table_list->next_global)
10271030
{
@@ -1049,7 +1052,8 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock,
10491052
{
10501053
found= FALSE;
10511054
/*
1052-
To avoid self and other kinds of deadlock we have to flush open HANDLERs.
1055+
To a self-deadlock or deadlocks with other FLUSH threads
1056+
waiting on our open HANDLERs, we have to flush them.
10531057
*/
10541058
mysql_ha_flush(thd);
10551059
DEBUG_SYNC(thd, "after_flush_unlock");
@@ -1308,8 +1312,7 @@ static void close_open_tables(THD *thd)
13081312

13091313

13101314
/**
1311-
Close all open instances of the table but keep the MDL lock,
1312-
if any.
1315+
Close all open instances of the table but keep the MDL lock.
13131316
13141317
Works both under LOCK TABLES and in the normal mode.
13151318
Removes all closed instances of the table from the table cache.
@@ -1323,6 +1326,8 @@ static void close_open_tables(THD *thd)
13231326
In that case the documented behaviour is to
13241327
implicitly remove the table from LOCK TABLES
13251328
list.
1329+
1330+
@pre Must be called with an X MDL lock on the table.
13261331
*/
13271332

13281333
void
@@ -1331,6 +1336,8 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
13311336
{
13321337
char key[MAX_DBKEY_LENGTH];
13331338
uint key_length= share->table_cache_key.length;
1339+
const char *db= key;
1340+
const char *table_name= db + share->db.length + 1;
13341341

13351342
memcpy(key, share->table_cache_key.str, key_length);
13361343

@@ -1352,8 +1359,6 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
13521359
*/
13531360
mysql_lock_remove(thd, thd->lock, table);
13541361

1355-
/* Make sure the table is removed from the cache */
1356-
table->s->version= 0;
13571362
/* Inform handler that table will be dropped after close */
13581363
if (table->db_stat) /* Not true for partitioned tables. */
13591364
table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
@@ -1365,7 +1370,14 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
13651370
prev= &table->next;
13661371
}
13671372
}
1368-
/* We have been removing tables from the table cache. */
1373+
/* Remove the table share from the cache. */
1374+
mysql_mutex_lock(&LOCK_open);
1375+
tdc_remove_table(thd, TDC_RT_REMOVE_ALL, db, table_name);
1376+
mysql_mutex_unlock(&LOCK_open);
1377+
/*
1378+
There could be a FLUSH thread waiting
1379+
on the table to go away. Wake it up.
1380+
*/
13691381
broadcast_refresh();
13701382
}
13711383

sql/sql_handler.cc

+29
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,35 @@ void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables)
833833
}
834834

835835

836+
/**
837+
Close cursors of matching tables from the HANDLER's hash table.
838+
839+
@param thd Thread identifier.
840+
@param tables The list of tables to flush.
841+
*/
842+
843+
void mysql_ha_flush_tables(THD *thd, TABLE_LIST *all_tables)
844+
{
845+
DBUG_ENTER("mysql_ha_flush_tables");
846+
847+
for (TABLE_LIST *table_list= all_tables; table_list;
848+
table_list= table_list->next_global)
849+
{
850+
TABLE_LIST *hash_tables= mysql_ha_find(thd, table_list);
851+
/* Close all aliases of the same table. */
852+
while (hash_tables)
853+
{
854+
TABLE_LIST *next_local= hash_tables->next_local;
855+
if (hash_tables->table)
856+
mysql_ha_close_table(thd, hash_tables);
857+
hash_tables= next_local;
858+
}
859+
}
860+
861+
DBUG_VOID_RETURN;
862+
}
863+
864+
836865
/**
837866
Flush (close and mark for re-open) all tables that should be should
838867
be reopen.

sql/sql_handler.h

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ bool mysql_ha_close(THD *thd, TABLE_LIST *tables);
2828
bool mysql_ha_read(THD *, TABLE_LIST *,enum enum_ha_read_modes,char *,
2929
List<Item> *,enum ha_rkey_function,Item *,ha_rows,ha_rows);
3030
void mysql_ha_flush(THD *thd);
31+
void mysql_ha_flush_tables(THD *thd, TABLE_LIST *all_tables);
3132
void mysql_ha_rm_tables(THD *thd, TABLE_LIST *tables);
3233
void mysql_ha_cleanup(THD *thd);
3334
void mysql_ha_move_tickets_after_trans_sentinel(THD *thd);

0 commit comments

Comments
 (0)