Hi Georgi,
On 2/17/09 3:10 PM, Georgi Kodinov wrote:
> #At file:///home/kgeorge/mysql/work/B42419-5.0-bugteam/ based on
> revid:matthias.leich@stripped
>
> 2742 Georgi Kodinov 2009-02-17
> Bug #42419: Server crash with "Pure virtual method called" on two concurrent
> connections
>
> The problem is that tables can enter open table cache for a thread without
> being properly
> cleaned up. This can happen if make_join_statistics() fails to read a const
> table because
> of e.g. a deadlock. It does set a member of TABLE structure to a value it
> allocates,
> but doesn't clean-up this setting on error nor does it set the rest of the
> members in
> JOIN to allow for automatic cleanup.
> As a result when such an error occurs and the next statement depends re-uses
> the table from
> the open tables cache it will get it with this TABLE::reginfo.join_tab
> pointing to a
> memory area that's freed.
> Fixed by making sure make_join_statistics() cleans up TABLE::reginfo.join_tab
> on error.
> modified:
> mysql-test/r/innodb_mysql.result
> mysql-test/t/innodb_mysql.test
> sql/sql_select.cc
>
[..]
> === modified file 'mysql-test/t/innodb_mysql.test'
> --- a/mysql-test/t/innodb_mysql.test 2008-11-27 14:54:23 +0000
> +++ b/mysql-test/t/innodb_mysql.test 2009-02-17 18:10:26 +0000
> @@ -1025,4 +1025,73 @@ CREATE INDEX i1 on t1 (a(3));
> SELECT * FROM t1 WHERE a = 'abcde';
> DROP TABLE t1;
>
> +#
> +# Bug #42419: Server crash with "Pure virtual method called" on two
> +# concurrent connections
> +#
> +
> +connect (c1, localhost, root,,);
> +connect (c2, localhost, root,,);
> +
> +CREATE DATABASE systest1;
> +USE systest1;
> +
> +CREATE TABLE tb1_eng1 (
> + i1 INT NOT NULL AUTO_INCREMENT, PRIMARY KEY (i1),
> + f1 INT
> +) ENGINE = InnoDB;
> +
> +INSERT INTO systest1.tb1_eng1 VALUES (1,1),(2,2),(3,3);
> +COMMIT;
> +
> +connection c1;
> +
> +SET AUTOCOMMIT = 0;
> +
> +CREATE TEMPORARY TABLE systest1.t1_tmp ( f1 INT );
> +
> +INSERT INTO systest1.t1_tmp (f1)
> + SELECT f1 FROM systest1.tb1_eng1 WHERE i1 = 3;
> +INSERT INTO systest1.t1_tmp (f1)
> + SELECT f1 FROM systest1.tb1_eng1 WHERE i1 = 2;
> +
> +
> +connection c2;
> +
> +SET AUTOCOMMIT = 0;
> +USE systest1;
> +
> +CREATE TEMPORARY TABLE systest1.t2_tmp ( i1 int, new_i1 int );
> +INSERT INTO systest1.t2_tmp VALUES
> +(1,51),(2,52),(3,53);
> +
> +UPDATE systest1.tb1_eng1 target
> +SET i1 = (SELECT new_i1 FROM systest1.t2_tmp source WHERE source.i1 = target.i1)
> + WHERE i1 = 1;
> +
> +--send
> +UPDATE systest1.tb1_eng1 target
> +SET i1 = (SELECT new_i1 FROM systest1.t2_tmp source WHERE source.i1 = target.i1)
> + WHERE i1 = 2;
> +
> +--sleep 2
> +
> +connection c1;
> +
> +--error ER_LOCK_DEADLOCK
> +INSERT INTO systest1.t1_tmp (f1)
> + SELECT f1 FROM systest1.tb1_eng1 WHERE i1 = 1;
> +
> +connection c2;
> +
> +--reap
> +UPDATE systest1.tb1_eng1 target
> +SET i1 = (SELECT new_i1 FROM systest1.t2_tmp source WHERE source.i1 = target.i1)
> + WHERE i1 = 3;
> +
> +connection default;
> +disconnect c1;
> +disconnect c2;
> +drop database systest1;
> +
Could you rewrite the test case so it doesn't need a different database
and it's more readable? Also, when merging to 5.1, please replace the
sleep with a wait condition.
> --echo End of 5.0 tests
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2009-02-05 09:30:39 +0000
> +++ b/sql/sql_select.cc 2009-02-17 18:10:26 +0000
> @@ -2415,10 +2415,10 @@ make_join_statistics(JOIN *join, TABLE_L
> table_vector[i]=s->table=table=tables->table;
> table->pos_in_table_list= tables;
> error= table->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK);
> - if(error)
> + if (error)
> {
> - table->file->print_error(error, MYF(0));
> - DBUG_RETURN(1);
> + table->file->print_error(error, MYF(0));
> + goto error;
Hum, if we jump here it's possible that some elements of the
table_vector array won't be initialized...
[..]
> +error:
> + /*
> + Need to clean up join_tab from TABLEs in case of error.
> + They won't get cleaned up by JOIN::cleanup() because JOIN::join_tab
> + may not be assigned yet by this function (which is building join_tab).
> + Dangling TABLE::reginfo.join_tab may cause part_of_refkey to choke.
> + */
> + for (uint inx= 0; inx < table_count; inx ++)
> + table_vector[inx]->reginfo.join_tab= NULL;
> + DBUG_RETURN (1);
But here we expect up to table_count to be initialized. I think it would
be better to iterate over the tables list.
Regards,
-- Davi Arnaut
