Hello Davi!
As always great work! I have only a couple of minor comments:
* Davi Arnaut <davi@stripped> [07/11/28 20:47]:
> ChangeSet@stripped, 2007-11-28 15:46:21-02:00, davi@stripped +10 -0
> Bug#23713 LOCK TABLES + CREATE TRIGGER + FLUSH TABLES WITH READ LOCK = deadlock
>
> This bug is actually two bugs in one, one of which is CREATE TRIGGER under
> LOCK TABLES and the other is CREATE TRIGGER under LOCK TABLES simultaneous
> to a FLUSH TABLES WITH READ LOCK (global read lock). Both situations could
> lead to a server crash or deadlock.
>
...
> diff -Nrup a/sql/sql_trigger.cc b/sql/sql_trigger.cc
> --- a/sql/sql_trigger.cc 2007-11-18 17:28:35 -02:00
> +++ b/sql/sql_trigger.cc 2007-11-28 15:46:19 -02:00
> @@ -292,6 +292,41 @@ private:
> };
>
>
> +/**
> + Exclusively name-lock a table that is already write-locked by the
> + current thread.
> +
> + @param thd current thread context
> + @param tables able list containing one table to open.
> +
> + @return FALSE on success, TRUE otherwise.
> +*/
> +
> +static bool name_lock_locked_table(THD *thd, TABLE_LIST *tables)
> +{
> + DBUG_ENTER("name_lock_locked_table");
May be it makes sense to move this function to sql_base.cc? We might
want to re-use it from places other than sql_trigger.cc in future and
sql_base.cc is more appropriate place for such functions, IMO.
> @@ -489,13 +517,20 @@ bool mysql_create_or_drop_trigger(THD *t
> /* Under LOCK TABLES we must reopen the table to activate the trigger. */
> if (!result && thd->locked_tables)
> {
> - /*
> - Must not use table->s->db.str or table->s->table_name.str here.
> - The strings are used in a loop even after the share may be freed.
> - */
> + /* Make table suitable for reopening */
> close_data_files_and_morph_locks(thd, tables->db, tables->table_name);
> thd->in_lock_tables= 1;
May be we should consider leaving original comment in place as well. After
all it still makes sense and its author has thought that it is necessary...
What do you think ?
I think it is OK to push your patch after considering above points.
--
Dmitri Lenev, Software Developer
MySQL AB, www.mysql.com
Are you MySQL certified? http://www.mysql.com/certification
