List:Commits« Previous MessageNext Message »
From:Libing Song Date:July 4 2010 7:40am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3423)
Bug#54197
View as plain text  
Hi Luis:
Nice work, patch approved.

Note: Someone has added another 'DBUG_VOID_RETURN' in the function,
please check it before you push it.

On Mon, 2010-06-21 at 00:05 +0000, Luis Soares wrote:
> #At file:///home/lsoares/Workspace/bzr/work/bugfixing/54197/mysql-5.1-bugteam/ based
> on revid:georgi.kodinov@stripped
> 
>  3423 Luis Soares	2010-06-21
>       BUG#54197: 'BINLOG ' statements crash the server
>       
>       Some BINLOG statements would make the server hit an assertion,
>       because they would lead to setting thd->main_da.m_status
>       twice. This was caused because the execution of some BINLOG
>       statements (in base 64 output) make my_ok to be called before
>       returning from ev->apply_event. Then, my_ok would be called
>       again, right before returning from mysql_client_binlog_statement,
>       triggering the assertion.
>       
>       We fix this by:
>       
>       1. moving the call to my_ok from inside
>          mysql_client_binlog_statement function, to
>          mysql_execute_command
>       
>       2. changing the signature of mysql_client_binlog_statement so
>          that it returns non-zero on error and zero otherwise.
>       
>       3. calling my_ok in mysql_execute_command conditionally, ie, if
>          the status hasn't been set yet.
> 
>     modified:
>       mysql-test/r/mysqlbinlog_base64.result
>       mysql-test/t/mysqlbinlog_base64.test
>       sql/mysql_priv.h
>       sql/sql_binlog.cc
>       sql/sql_parse.cc
> === modified file 'mysql-test/r/mysqlbinlog_base64.result'
> --- a/mysql-test/r/mysqlbinlog_base64.result	2008-07-28 07:15:20 +0000
> +++ b/mysql-test/r/mysqlbinlog_base64.result	2010-06-21 00:05:45 +0000
> @@ -109,3 +109,9 @@ count(*)
>  35840
>  drop table t1;
>  drop table t2;
> +RESET MASTER;
> +CREATE TABLE t1(c1 INT) ENGINE=myisam;
> +FLUSH LOGS;
> +INSERT INTO t1 VALUES(1),(2);
> +DROP TABLE t1;
> +DROP TABLE t1;
> 
> === modified file 'mysql-test/t/mysqlbinlog_base64.test'
> --- a/mysql-test/t/mysqlbinlog_base64.test	2008-07-28 07:15:20 +0000
> +++ b/mysql-test/t/mysqlbinlog_base64.test	2010-06-21 00:05:45 +0000
> @@ -71,3 +71,23 @@ select count(*) from t2;
>  --remove_file $MYSQLTEST_VARDIR/tmp/mysqlbinlog_base64.sql
>  drop table t1;
>  drop table t2;
> +
> +#
> +# BUG#54197 'BINLOG ' statements crash the server
> +#
> +
> +RESET MASTER;
> +
> +CREATE TABLE t1(c1 INT) ENGINE=myisam;
> +FLUSH LOGS;
> +INSERT INTO t1 VALUES(1),(2);
> +
> +-- let $fname_prefix= `SELECT UUID()`;
> +-- let $outfile= $MYSQL_TEST_DIR/var/tmp/$fname_prefix.sql
> +
> +--let $MYSQLD_DATADIR=`select @@datadir`
> +--exec $MYSQL_BINLOG --base64-output $MYSQLD_DATADIR/master-bin.000001 >
> $outfile
> +DROP TABLE t1;
> +--exec $MYSQL test < $outfile
> +-- remove_file $outfile
> +DROP TABLE t1;
> 
> === modified file 'sql/mysql_priv.h'
> --- a/sql/mysql_priv.h	2010-05-27 20:07:40 +0000
> +++ b/sql/mysql_priv.h	2010-06-21 00:05:45 +0000
> @@ -1045,7 +1045,7 @@ bool mysql_alter_db(THD *thd, const char
>  bool mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent);
>  bool mysql_upgrade_db(THD *thd, LEX_STRING *old_db);
>  void mysql_binlog_send(THD* thd, char* log_ident, my_off_t pos, ushort flags);
> -void mysql_client_binlog_statement(THD *thd);
> +int mysql_client_binlog_statement(THD *thd);
>  bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists,
>                      my_bool drop_temporary);
>  int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
> 
> === modified file 'sql/sql_binlog.cc'
> --- a/sql/sql_binlog.cc	2009-10-14 01:39:05 +0000
> +++ b/sql/sql_binlog.cc	2010-06-21 00:05:45 +0000
> @@ -31,7 +31,7 @@
>    statement.
>  */
>  
> -void mysql_client_binlog_statement(THD* thd)
> +int mysql_client_binlog_statement(THD* thd)
>  {
>    DBUG_ENTER("mysql_client_binlog_statement");
>    DBUG_PRINT("info",("binlog base64: '%*s'",
> @@ -40,7 +40,7 @@ void mysql_client_binlog_statement(THD* 
>                       thd->lex->comment.str));
>  
>    if (check_global_access(thd, SUPER_ACL))
> -    DBUG_VOID_RETURN;
> +    DBUG_RETURN(1);
>  
>    size_t coded_len= thd->lex->comment.length + 1;
>    size_t decoded_len= base64_needed_decoded_length(coded_len);
> @@ -56,7 +56,7 @@ void mysql_client_binlog_statement(THD* 
>      Format_description_event.
>    */
>    my_bool have_fd_event= TRUE;
> -  int err;
> +  int err= 0;
>    Relay_log_info *rli;
>    rli= thd->rli_fake;
>    if (!rli)
> @@ -85,6 +85,7 @@ void mysql_client_binlog_statement(THD* 
>          rli->relay_log.description_event_for_exec &&
>          buf))
>    {
> +    err= 1;
>      my_error(ER_OUTOFMEMORY, MYF(0), 1);  /* needed 1 bytes */
>      goto end;
>    }
> @@ -111,6 +112,7 @@ void mysql_client_binlog_statement(THD* 
>  
>      if (bytes_decoded < 0)
>      {
> +      err= 1;
>        my_error(ER_BASE64_DECODE_ERROR, MYF(0));
>        goto end;
>      }
> @@ -150,6 +152,7 @@ void mysql_client_binlog_statement(THD* 
>                            event_len, bytes_decoded));
>        if (bytes_decoded < EVENT_LEN_OFFSET || (uint) bytes_decoded <
> event_len)
>        {
> +        err= 1;
>          my_error(ER_SYNTAX_ERROR, MYF(0));
>          goto end;
>        }
> @@ -166,6 +169,7 @@ void mysql_client_binlog_statement(THD* 
>            have_fd_event= TRUE;
>          else
>          {
> +          err= 1;
>            my_error(ER_NO_FORMAT_DESCRIPTION_EVENT_BEFORE_BINLOG_STATEMENT,
>                     MYF(0), Log_event::get_type_str((Log_event_type)type));
>            goto end;
> @@ -183,6 +187,7 @@ void mysql_client_binlog_statement(THD* 
>            causes by a bad statement
>          */
>          my_error(ER_SYNTAX_ERROR, MYF(0));
> +        err= 1;
>          goto end;
>        }
>  
> @@ -238,10 +243,9 @@ void mysql_client_binlog_statement(THD* 
>  
> 
>    DBUG_PRINT("info",("binlog base64 execution finished successfully"));
> -  my_ok(thd);
>  
>  end:
>    rli->clear_tables_to_lock();
>    my_free(buf, MYF(MY_ALLOW_ZERO_PTR));
> -  DBUG_VOID_RETURN;
> +  DBUG_RETURN(err);
>  }
> 
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc	2010-06-07 10:01:54 +0000
> +++ b/sql/sql_parse.cc	2010-06-21 00:05:45 +0000
> @@ -4922,7 +4922,15 @@ create_sp_error:
>    case SQLCOM_BINLOG_BASE64_EVENT:
>    {
>  #ifndef EMBEDDED_LIBRARY
> -    mysql_client_binlog_statement(thd);
> +    if (!mysql_client_binlog_statement(thd) && !thd->main_da.is_set())
> +      /* 
> +        Applying some 'BINLOG' statements makes my_ok to be called
> +        right after applying the event (ev->apply_event). So we
> +        need to check if the thd status hasn't yet been set, before
> +        we set it here, thence not risking setting OK twice.
> +        See BUG#54197.
> +      */
> +      my_ok(thd);
>  #else /* EMBEDDED_LIBRARY */
>      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "embedded");
>  #endif /* EMBEDDED_LIBRARY */
> 
> text/bzr-bundle type attachment
> (bzr/luis.soares@stripped)
> # Bazaar merge directive format 2 (Bazaar 0.90)
> # revision_id: luis.soares@stripped
> # target_branch: file:///home/lsoares/Workspace/bzr/work/bugfixing\
> #   /54197/mysql-5.1-bugteam/
> # testament_sha1: 424103cf0b79c82957a3c226e50fac80d5fd951c
> # timestamp: 2010-06-21 01:05:52 +0100
> # source_branch: /home/lsoares/Workspace/bzr/clones/mysql-5.1-bugteam
> # base_revision_id: georgi.kodinov@stripped\
> #   w1hfw1unm43r4b14
> # 
> # Begin bundle
> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWbCX74UABULfgHUQfff//3/v
> /6D////wYAsO+2O9x49deRNgtHes9aZToodeh7ttqVV33ASSISZppoU8pvVNqexGqH6p5T1PaRG1
> PUGmgD1PUaAPUEopoA9TKYCnpR5T1DeqaD1AAGIAAADQSSJgp6aFPJNU9Mk9TA0BoCMn6gE2EEMh
> tEEiImiKn+lH6p6nip6eqep5T0njVPKepvSmmj1NGgZG0gYQ0GVIA0eUAAAAAaAAAAAAASSAgTTJ
> ggTTRGJ6mTKepPSNAGjR6gaGj01IliYE1G+n28sCl+78hnz5u/yDMHMycDfbs/PDjLWV+BXr+M82
> yknbg4eg9Umri2PTZ/tIXJPudEdyIuLDRGxmPRwLBhBdGowjuIP8XRiWRjbS6A+U3E5aqfss/HNJ
> amElaQhg08Mo2x2y7mfNIVY4ssUva6eTxovIrQzMzSbGxN+zu8olRmaMGbGbZjgmk89IiYm9F82e
> 27hphFThwKTDG6Mi/TNqxDq7Pv2rjpiMDke34Sp6DyjP55M3lP0CStjXibjg0P5udupzxGNwZ4d1
> ZK29kwH55rIllmFZiSlaL5SPzGEVEIM57U6VDO1dNCecdSdPcijKOp4r3PCZaj7n22rsw3OY3xcl
> hGcfBUHf4M6C7fg4u7hJtb4zYoe6/GRdcleqvVq0brm6NxYTRJ1muRDRvgs0oImSR0qpYUbmRtZP
> ZVjS+Tgb6SA5x8SfP2+fmd0EM9sdPDZA58OGXqkwXDPW8ZpTYuVWmU5SZQwkZ0CRE9y6xvyMjmJa
> qfzbaaSJ0s2M/yFWXTKsoKIQOi1W15tB1mbO7uOydGxvhNMZEWeDklkvx6A0sV5JLzcvTz580V79
> SP83I2u2Y0CIsAbmSy5xImuMz88xLaD9xeKKiWBAglUHBvhIKhKK4RLKlhwSj0JDyCsek8TADFI4
> OYwlzKibUBoJQqxiVUySuV/CFnJVWUyAxEkRHRUC46xiJW0pKMnIodBm+687A6LHw+N4ljekDDB+
> zC8eFyZaC8KltpInxjlaQzEaka7HgdsmfWPRXC0GiJmOTpasIXpTg0HQFjK4uqtCo3afncNlN0Xx
> TG0FfNgLsJUGM0UG8cbtqNxmcHiPosWXG7XddyOWyJgKTCRpoYC0LMhJ9gwtzheRIlrJGM7eIxAz
> ulApNDbURUtT57BqzXapwgb6AL9DWV/P6FZdI3H2mGBsQFd7Pjjc5dxrtDhMDCSwxJ48smHCzgop
> eQHbd28cSizLRZtJENRgVFrkz5tCdK7U2HBxDsoFr4jKxUp6Q0cgcs4p7g1wnQr1FqwgFBqtUSg4
> qMXUgXkbGJj9RYE1UVi2pxwN5aR2kXNSNvHMXDGY7RcTuJEL1dSMO0e8hEfEUBLCpz0SccCIpLHW
> 5aQfIcK44Kon9yiAXmkHJDxs3FJv1BMcDF/DUd5q3Q9DwITHGvlVx0hLW1EayRcGgxGiiggJbolV
> yUCRpkYHrkdeyRUVZ0XwLrhw92FdNowoYnCM4fLWpmRW/U2kqJ6OLuZ2sZOJ1o8vTCUMnSM8fVRX
> USGFFxMLqjBmgiWkw8VTEuCkhYrWGU7wgYjrgcbr7nrAkThKlqlB8S4lOeOBY0sWmNXMpnezWcEg
> uhNGeLiWmviKEU1c6XUC81WslritwhhoV0J1r38K4wWY0mL3Wfo6YOUoPeJoMk5dBXgXWfXcdUQD
> Vo/V0dOQ7DV0Ce3cLk4sKJtmY2ImemEMXfWZNqI9BKw76yFaNO1JbgP4fo9BoP3yNibGDbx9B74j
> 1ig6sDCw+33PA/q/CXYO0yGFyHAYv6tCI46CI944mfNEE1BhxAiO+B5RSKBmp9CgLSoomJ0ETduP
> sjI2FBgWRmBGAag2IkTG+ZeZGJgEgxPHEeRPojIfwuK7iBahAUgxMPiQJF0TM3kYax5crfmh+hDQ
> fyi9C4xMXCfOdhJM0RCFQVFA1Py+dwIsLpjf15f31lAXlP29RtLO5vObgbeT1mQThxtny1WfDKYG
> zYjn/hHBEZK+/VSY6mz8v9LsduiFygDL484F5qJl45q2SHa3L+exSP3HS44G0/gJWH3r5QLzKg6l
> gcjGv1F2lpbEOtkYE5rvEwnlRDsaXbzKUkTOy5wXffU76yL+fKbDjeGNhzlZ4owGkcR8xRpobNR1
> ZHM7rvqdYwwg4W4VqSGMJrtJ1Tcub73nFh7OJOlCMBXJdXDkbqPebT3nPePMuFvIuSpeakqiGqml
> IzM/6Vld8ye4wYyo3mgWkIb8RflmI7VPA8VXeoTmqoYkpJ0xCjjiopIqtlVFtcXSGVCDboNvJHL1
> +GqXIo9KTYWJex5DyPHjd5I2o112CZxRJFIEiReQzn5m4fB6YubOzAefbodfu7c5tfaahLiqR66F
> HWb++7i3d9TjLRIseaGeTChmhdmE5Dk9DkqjKGe2JRFxmwUOu671uEs19nX+Tf1h50or2cmSWWVw
> M/NZUA1w4grrOLepACgS5DyFaRbYxqPMbuEdD2hvOK6zzO8idx6EGITPEgPJ0icFYuC3F6w+aPeh
> +QrykXyN71A87xyrNcH6KaRom8jm8FPh1OQUh3ZeLEQq9z9qp6HTxXKJynkXiKDkE9Rt8h4CEBpr
> 903MocyilNXeZnPGYc0gahRBivNiQ7PLtSL3augqXeTdgqVPMESI9hdnWU3AXSwK498bYuvpREgD
> FRXxa3b2S1VRFn6GgyS67TdcBfLWUVwWasRj7jnmazUUHgdUjrOvvZkM6w3mZidRsTV8wJ4nfuDw
> IHeWDrTgX7WSwCokBuI2RCSSNF4IKgZI4Rmok/bEoEqBSwOTBQ3sSTpEfwPiJkdvs438CfiHfE5o
> F8C1eNxDl9Zuq8y7C7clMDfSNOAvponEkyjXKlBVUeofoSR5NpbsVA97W3jBsbPGadDybyPCREwD
> 4wCBQMpRicZUTZTAn8Wi8FAryHpKaqHBSnngRCE2ZhhIZgTJjjv3sGCI3ntOxvruQmktQUxOukug
> Urcki4tCuzh4WgnptzDNoZAd5aQarKpq093oMei+IaSmGhIYwSVxUS5shkslIUJq0GPgPO/iuVkX
> zkadCYl4iThN2JkklvEmcfEdRsVJuPhaWmTGPVEfjBy8VEXOYL2dh9RsLCJtaqR6l51WoSu9XnFz
> hUUg2zHwjBql7Sa1rWWpplMlVRjUDX3VCztaxws2adZdJQdMay0lKSiOL6NcmONqqiME9J/yyoME
> XspEEe1oI1xqAejgO1nbelwrL/AkmdKN7bbbbnQt4TBAEQQkfIarzvkIuFyrBzEcAI+8Ad1GZfVJ
> Z0R592n4TfMg+bx/J11c3VmmqzsjWZTLyA5Tm0USMNwhy0l0TnNlUi+WFT4EQgs4RJ4P0vdcCXqc
> aM7TWMUu1rnkTYm5guwBAKoVyzBIYgoUJSEpCQHK4qFDRkMSCtOqLaIhJ2lq7DlogGsTx91qTL2n
> kbikGY7FkZqQ6AcMMR8FtW5YeyMQMT7BLDojNeJ1HrhqNBvQithkWiUC+yF5USR6iW4MSggkWM5x
> 1OLhOrtIECs2DkkcD4jyzhcKqqg7SBSV2sQ1vOZ/8XckU4UJCwl++FA=
> 
> 

-- 
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer


Email : Li-Bing.Song@stripped
Skype : libing.song
MSN   : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================

Thread
bzr commit into mysql-5.1-bugteam branch (luis.soares:3423) Bug#54197Luis Soares21 Jun
  • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3423)Bug#54197Libing Song4 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3423)Bug#54197Alfranio Correia14 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3423) Bug#54197Luís Soares22 Jul
      • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3423) Bug#54197Alfranio Correia30 Jul