Hi Andrei!
I think there is a problem with this patch. It introduces a way for the
assignment operator and the copy constructor to fail. The copy
constructor is never used, but the assignment operator is used in one
place in the server (namely, in sql_class.cc, at the top of
THD::reset_sub_statement_state). This place does not check for the failure:
if (rpl_master_erroneous_autoinc(this))
{
backup->auto_inc_intervals_forced= auto_inc_intervals_forced;
auto_inc_intervals_forced.empty();
}
It looks as if it is actually not necessary to allocate more dynamic
memory at this point. So I'd suggest to just copy the list pointers from
auto_inc_intervals_forced to backup->auto_inc_intervals_forced, then
call auto_inc_intervals_forces.empty_no_free(). Copy the list pointers
should be done by a new member function.
After that, I think we can throw away the copy constructor and
assignment operator, since they have the problem that they can fail
(which is quite unexpected for an assignment) and since they are not
used any more.
Andrei Elkin wrote:
> #At file:///home/andrei/MySQL/BZR/FIXES/bug36443-slave_crash_insert_trigger/
>
> 2656 Andrei Elkin 2008-06-17
> Bug #36443 Server crashes when executing insert when insert trigger on table
>
> The crash appeared to be a result of allocating an instance of
> Discrete_interval
> automatically whereas it meant to be dynamical.
>
> Fixed with correcting the allocation introduced by bug@33029;
> added simulation code that forces executing those fixes of bug@33029 that
> targeted
> at master-and-slave having incompatible bug33029-prone versions.
> added:
> mysql-test/suite/bugs/r/rpl_bug33029.result
> mysql-test/suite/bugs/t/rpl_bug33029.test
> modified:
> sql/slave.cc
> sql/structs.h
>
> per-file messages:
> mysql-test/suite/bugs/r/rpl_bug33029.result
> new results file
> mysql-test/suite/bugs/t/rpl_bug33029.test
> test merely checks no crash happens on slave.
> sql/slave.cc
> forcing to execute special logics implemented for bug#33029 if
> simulate_bug33029 the debug option is set.
> sql/structs.h
> converting automatic allocation of a being appended item to Discrete_interval to
> dynamic.
> === added file 'mysql-test/suite/bugs/r/rpl_bug33029.result'
> --- a/mysql-test/suite/bugs/r/rpl_bug33029.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/bugs/r/rpl_bug33029.result 2008-06-17 15:32:36 +0000
> @@ -0,0 +1,15 @@
> +stop slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +reset master;
> +reset slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +start slave;
> +create table `t1` (`id` int not null auto_increment primary key);
> +create trigger `trg` before insert on `t1` for each row begin end;
> +set @@global.debug="+d,simulate_bug33029";
> +stop slave;
> +start slave;
> +insert into `t1` values ();
> +select * from t1;
> +id
> +1
>
> === added file 'mysql-test/suite/bugs/t/rpl_bug33029.test'
> --- a/mysql-test/suite/bugs/t/rpl_bug33029.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/bugs/t/rpl_bug33029.test 2008-06-17 15:32:36 +0000
> @@ -0,0 +1,25 @@
> +#
> +# Bug #36443 Server crashes when executing insert when insert trigger on table
> +#
> +# Emulating the former bug#33029 situation to see that there is no crash anymore.
> +#
> +
> +
> +source include/master-slave.inc;
> +
> +create table `t1` (`id` int not null auto_increment primary key);
> +create trigger `trg` before insert on `t1` for each row begin end;
> +
> +sync_slave_with_master;
> +set @@global.debug="+d,simulate_bug33029";
> +
> +stop slave;
> +start slave;
> +
> +connection master;
> +
> +insert into `t1` values ();
> +
> +sync_slave_with_master;
> +select * from t1;
> +
>
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc 2008-03-31 08:57:18 +0000
> +++ b/sql/slave.cc 2008-06-17 15:32:36 +0000
> @@ -4136,6 +4136,7 @@ bool rpl_master_erroneous_autoinc(THD *t
> if (active_mi && active_mi->rli.sql_thd == thd)
> {
> Relay_log_info *rli= &active_mi->rli;
> + DBUG_EXECUTE_IF("simulate_bug33029", return TRUE;);
> return rpl_master_has_bug(rli, 33029, FALSE);
> }
> return FALSE;
>
> === modified file 'sql/structs.h'
> --- a/sql/structs.h 2008-03-14 03:35:41 +0000
> +++ b/sql/structs.h 2008-06-17 15:32:36 +0000
> @@ -315,13 +315,24 @@ private:
> Discrete_interval *current;
> uint elements; // number of elements
>
> - /* helper function for copy construct and assignment operator */
> + /*
> + helper function for copy construct and assignment operator
> + @note if dynamic allocation fails the target list is nullified
> + */
> void copy_(const Discrete_intervals_list& from)
> {
> for (Discrete_interval *i= from.head; i; i= i->next)
> {
> - Discrete_interval j= *i;
> - append(&j);
> + Discrete_interval *j= new Discrete_interval(*i);
> + if (j == NULL)
> + {
> + empty();
> + break;
> + }
> + else
> + {
> + append(j);
> + }
> }
> }
> public:
>
>
--
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com
