Hi Luis,
Thank you for fixing the problem, patch approved!
Luis Soares wrote:
> #At file:///home/lsoares/Workspace/bzr/work/bugfixing/55387/mysql-trunk-bugfixing/
> based on revid:jonathan.perkin@stripped
>
> 3133 Luis Soares 2010-07-22
> BUG#55387: binlog.binlog_tmp_table crashes the server
> sporadically
>
> There are two problems:
>
> 1. When closing temporary tables, during the THD clean up - and
> after the session connection was already closed, there is a
> chance we can push an error into the THD diagnostics area, if
> the writing of the implicit DROP event to the binary log fails
> for some reason. As a consequence an assertion can be
> triggered, because at that point the diagnostics area is
> already set.
>
> 2. Using push_warning with MYSQL_ERROR::WARN_LEVEL_ERROR is a
> bug.
>
> Given that close_temporary_tables is mostly called from
> THD::cleanup - ie, with the session already closed, we fix
> problem #1 by allowing the diagnostics area to be
> overwritten. There is one other place in the code that calls
> close_temporary_tables - while applying Start_log_event_v3. To
> cover that case, we make close_temporary_tables to return the
> error, thus, propagating upwards in the stack.
>
> To fix problem #2, we replace push_warning with sql_print_error.
> @ sql/log_event.cc
> Added handling of error returned by close_temporary_tables to
> Start_log_event_v3::do_apply_event.
> @ sql/sql_base.cc
> Three changes to close_temporary_tables:
> 1. it returns a boolean now (instead of void)
> 2. it uses sql_print_error instead of push_warning when writing to
> binary log fails
> 3. we set can_overwrite_status before writing to the binary log,
> thence not risking triggering an assertion by any other push
> into diagnostics area happening inside mysql_bin_log.write.
> @ sql/sql_base.h
> Changed the interface of close_temporary_tables so that it returns
> bool instead of void.
>
> modified:
> sql/log_event.cc
> sql/sql_base.cc
> sql/sql_base.h
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc 2010-07-15 13:47:50 +0000
> +++ b/sql/log_event.cc 2010-07-22 15:56:50 +0000
> @@ -3719,6 +3719,7 @@ bool Start_log_event_v3::write(IO_CACHE*
> int Start_log_event_v3::do_apply_event(Relay_log_info const *rli)
> {
> DBUG_ENTER("Start_log_event_v3::do_apply_event");
> + int error= 0;
> switch (binlog_version)
> {
> case 3:
> @@ -3731,7 +3732,7 @@ int Start_log_event_v3::do_apply_event(R
> */
> if (created)
> {
> - close_temporary_tables(thd);
> + error= close_temporary_tables(thd);
> cleanup_load_tmpdir();
> }
> else
> @@ -3759,7 +3760,7 @@ int Start_log_event_v3::do_apply_event(R
> Can distinguish, based on the value of 'created': this event was
> generated at master startup.
> */
> - close_temporary_tables(thd);
> + error= close_temporary_tables(thd);
> }
> /*
> Otherwise, can't distinguish a Start_log_event generated at
> @@ -3771,7 +3772,7 @@ int Start_log_event_v3::do_apply_event(R
> /* this case is impossible */
> DBUG_RETURN(1);
> }
> - DBUG_RETURN(0);
> + DBUG_RETURN(error);
> }
> #endif /* defined(HAVE_REPLICATION) && !defined(MYSQL_CLIENT) */
>
>
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc 2010-07-15 13:47:50 +0000
> +++ b/sql/sql_base.cc 2010-07-22 15:56:50 +0000
> @@ -1634,7 +1634,7 @@ static inline uint tmpkeyval(THD *thd,
> creates one DROP TEMPORARY TABLE binlog event for each pseudo-thread
> */
>
> -void close_temporary_tables(THD *thd)
> +bool close_temporary_tables(THD *thd)
> {
> DBUG_ENTER("close_temporary_tables");
> TABLE *table;
> @@ -1642,9 +1642,10 @@ void close_temporary_tables(THD *thd)
> TABLE *prev_table;
> /* Assume thd->variables.option_bits has OPTION_QUOTE_SHOW_CREATE */
> bool was_quote_show= TRUE;
> + bool error= 0;
>
> if (!thd->temporary_tables)
> - DBUG_VOID_RETURN;
> + DBUG_RETURN(FALSE);
>
> if (!mysql_bin_log.is_open())
> {
> @@ -1655,7 +1656,7 @@ void close_temporary_tables(THD *thd)
> close_temporary(table, 1, 1);
> }
> thd->temporary_tables= 0;
> - DBUG_VOID_RETURN;
> + DBUG_RETURN(FALSE);
> }
>
> /* Better add "if exists", in case a RESET MASTER has been done */
> @@ -1754,11 +1755,27 @@ void close_temporary_tables(THD *thd)
> qinfo.db= db.ptr();
> qinfo.db_len= db.length();
> thd->variables.character_set_client= cs_save;
> - if (mysql_bin_log.write(&qinfo))
> +
> + thd->stmt_da->can_overwrite_status= TRUE;
> + if ((error= (mysql_bin_log.write(&qinfo) || error)))
> {
> - push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, MYF(0),
> - "Failed to write the DROP statement for temporary tables to
> binary log");
> + /*
> + If we're here following THD::cleanup, thence the connection
> + has been closed already. So lets print a message to the
> + error log instead of pushing yet another error into the
> + stmt_da.
> +
> + Also, we keep the error flag so that we propagate the error
> + up in the stack. This way, if we're the SQL thread we notice
> + that close_temporary_tables failed. (Actually, the SQL
> + thread only calls close_temporary_tables while applying old
> + Start_log_event_v3 events.)
> + */
> + sql_print_error("Failed to write the DROP statement for "
> + "temporary tables to binary log");
> }
> + thd->stmt_da->can_overwrite_status= FALSE;
> +
> thd->variables.pseudo_thread_id= save_pseudo_thread_id;
> thd->thread_specific_used= save_thread_specific_used;
> }
> @@ -1771,7 +1788,8 @@ void close_temporary_tables(THD *thd)
> if (!was_quote_show)
> thd->variables.option_bits&= ~OPTION_QUOTE_SHOW_CREATE; /* restore option
> */
> thd->temporary_tables=0;
> - DBUG_VOID_RETURN;
> +
> + DBUG_RETURN(error);
> }
>
> /*
>
> === modified file 'sql/sql_base.h'
> --- a/sql/sql_base.h 2010-06-11 15:28:18 +0000
> +++ b/sql/sql_base.h 2010-07-22 15:56:50 +0000
> @@ -222,7 +222,7 @@ int decide_logging_format(THD *thd, TABL
> void free_io_cache(TABLE *entry);
> void intern_close_table(TABLE *entry);
> bool close_thread_table(THD *thd, TABLE **table_ptr);
> -void close_temporary_tables(THD *thd);
> +bool close_temporary_tables(THD *thd);
> TABLE_LIST *unique_table(THD *thd, TABLE_LIST *table, TABLE_LIST *table_list,
> bool check_alias);
> int drop_temporary_table(THD *thd, TABLE_LIST *table_list);
>
> text/bzr-bundle 类型 附件
> (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\
> # /55387/mysql-trunk-bugfixing/
> # testament_sha1: 8b15a83382ff7cd6a410d78c4c936a9568c92589
> # timestamp: 2010-07-22 16:56:54 +0100
> # source_branch: /home/lsoares/Workspace/bzr/clones/mysql-trunk-\
> # bugfixing
> # base_revision_id: jonathan.perkin@stripped\
> # gqyrtmkgwrjfgsmc
> #
> # Begin bundle
> IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWevzfBIABGZ/gFWQBABZ9///
> f+f/oL////RgCofXvd7t3nvn275kO1rmeddu7a7Zbdd6PPedq220dXbwkkJGCmTYjRkNT0qf6poy
> nqaflTAjGo/UJtNBqZogyQEnhNTyTKTZQ0ep6m0QeoGmQAAAGjQRTBMUxNDQNAGgDQAAAAAAAkRB
> E1NPUxD1NGpojano0TTINBoAADQGgiiQp5MiaT0TGpkaNNNNMmmTJkDQaAABoJIppkaJpiBop4CR
> pqbRGg0A0DIAB6RjOthBdpq03OO9tcG/TRGmblYTztxf319g/C8I3yS7r33cc/ax5foXRfWmyl/Z
> 8PDNkqTa2pjtiz2O6tv6xwkzfNw8mckDQcWas3W+dvRWzUytlCLdOVJGfhXkhGTJCKUAMmZMwW5j
> To78nDzncV3LnWlMzAMzMLdjPokQr+0ZdHXHTN6nB8HDo4YWpVY0HZkyVdXdxW9Sl2q2y1EBiOy4
> MDK9yAIxVAKAKnqkaJQQl5fSkqiQBIDE5RKQsg/bR1bTVdOxqKqCL8KDlGFveaC6cRojZngxPO51
> dH/Rn8OCmQgBOKn1564CzsrBTEl2JyTPJyMEF/O5V+kK5MzBk4XjsTOd68xsdYm+F08rePK+y2Wn
> FWGWriagaAWWVRffSbIkahQlZObJ4WtKChRTB9ObPddZWaBcKwljwBKLsFWQMqRmXAhzJUQqNo4I
> hoGBnyXgiurtuzKb/dneeJYbU+luTclD+XIXnynGFBraB/ppGInp5r0Wu6LIS8S92JN5PRMUxT2Y
> ObCEjclGtpVctQti4AFqyEp0bz3zs26wRfrOwzYMYlpn1Hm+HSoq6cEZbRuCK36C6vZEsKQwEXss
> Ka8pLapWcuxndOzStaJlrHdgSIQnWBieMZRMzXCh1hoBS2Vo6BGbNKttIKNMVy/ShrIsXKDd67yy
> DBk5QEXmZ9XIF625iQLggPs5FA37cBXpxzCiFVIufui4Kkn7DJ22Dny6CdOM816K4c+XCOwYrEPk
> QMwFVcror6hatFM8FJ70bL02rxmbdSK/GsdMT3VUb95JYpAxlUI0eOCRZtYbE8JaGWJORcOaXHwI
> bIC4ETjM4cE6QLOc5rodEt8qyxukC94WNRs5wFdQIk9AjBpKzCfAoUIG9oLhWiziMm5WDC4EaStZ
> /pc1mNloosIwND53GsuHC8zO0Z8dtZG0d4nI2zCELS4oBvcuxC03SsjRa1FKqEOZEi4veowKuCg6
> 50qgRN1AREnBytPMBiVtJamDsVU5RvgQ3W2Gene2wbNcraXqTKXItICKaS2PO6R4h8aXYJdFlhSX
> 5y4qOm+1bBbeXbljHELcbmpTEeRXUmag0HBibm1UvEZ1KRqpjGTb1vOMmZMIxEm8p8xhGlMJBov3
> g9jqirljoM3VtgYmcq3JRY221NhTTTh0GWsvGWbAxotYxSKRswsNksdZjVlQaPeay0YRaPnNp5jN
> ZlEPn+k1b8nHP0XrBrgiMlz4PKFDtQJCLyiqiXAYOsQ04scbb8fBDMnYBq6VQMfvksTDbFsE4bbw
> XScGMSukJkjsX38tvHNlVCYZmhtCp3aLiYw7l9pPv+DwBxylW2pGfwFh4E/6WNGM8yDwMExUFoV6
> SJMtvUz3MXNnwHHYkDuZ2ev0omEIxf3WHs9oer4wXIX0eoDYduplCSWGt7nTUNb02gyDbU1vJohW
> REx4GoDkOA46ziOY5CSTkOUvPKWHs6squp541KHO5FhhqwOW9zRFxfeHxZL5J24tfZAcFnBR5w96
> hPKkMVETwyc/RKZUNoHpKm9wC8WmLDWEqx3mWwXM7ml5FMoQgpYHMT6l0XkI17CNdauH9FxpEXjq
> VKMoNGm/emJe4vMVb1P6GsaJA8zhyQJw9gjaeXtJbDoaC1dGX7jc7FqA2sXy362YY4nWkfoI7iiB
> NZstxkXMbktM5nkAxoFJaRYGU5d3FRLkY+OzUpm9RPVq1+DvXgsnwg7mcSOstkceSK0LJdFiUbzk
> L2LTLrDVmNYP5QzW91dcpBVkb9jsyH0FeVDw3+6SzBqlibuCTkpDziO/Fk2V7r2jwzNYsVnzTPeh
> yZDLXWS2zOvLn+8z3GI6VSvCVtys7KLEKCvBaA6lAOamCINtyJNDLCdEojwJFlPPsosyk1+oN0ce
> WGj2+KKG1JOOLR0t2ye1pbcxNNJowfLNk8efoigaRaNS752M9Lh+k8UxJ6y2C1huzanXBxMCCR2b
> xGhEhu9yMQtuQM4sEpVnal/z6uguwdkMDUpFsOc3dQ4iuqEZqFiIBNv8eYLU9PXeurmTLUjg+peV
> dqlbGW8CjfP5Fl3hthRpEHa1DQSbzmRlSrGRZmKg03iYdl7DeWuftxBwBjliikKNnsiO6iVRDElu
> v+Pooj1YRF6DjcuZVJfUAMshjQtvkDq0H4aaA1Fy7trIL3wi3e1gWHeXIW8B4cRcCNR7wKA4eJnd
> zythFtQ1zBBHmmuYvO5MyYunCjyccbLmGZhR3sFrpG4U+dKcWCYPP8u1TWsLeR56lesl3K7Jlit+
> F6gyrXIWeztNoMHFDDM3QMl6CQtUF7YpL0GU7PMc2DBMgg6nQXUoDGalwqyq507KhiNFdR7vAvg9
> QoAKCMKBxes69D5CYGWjCG6d3OtCptGQFOxVp9CiinCgsW2lZVJrUcsJAuxC4ODOs2NneuvujEwW
> kSziqaVVQRnFo07rURWVCbKK6gJVyqqqCdOUrFEkUNa13stVi694NWLIVkUqldsBpiYNNisQnyZn
> kYN7N9FwwqX0p+cYJFq4tnS3ID5jzHixClrJVIC6NJZhPG6HNFbspob7AXQMQp1OGYYFqCJV4BzW
> 8Pf1uZjF7u1eWispXFcGr1oGnSdXTAUBo8bXvNl1ljS2gtNoMVGvI5JDDckbOByuo/7GKZLWuPsX
> BbZbaNkEDGJppLgxLnpQ9jHFGFxsDCNEEFDAmDRpcKTVuH1XhT40KCtWtRsGh3WS1u1b6PZEMQha
> w8TfbRO+UrGCAdvMZHYlT5q84PO9/G8DKEoYqzKR1UoNIpwRyTTFIzJLNska4sEhDogF+hekROmv
> 8bLDOKpNci/Bda2qFsH7aRHH5153uiI9nKt8qkqsNzFC/reiMRm5SQ9q5goL6T5cheUSei1aYZS2
> oGSYxJMRRijecykXWvKKFxizYm8l6Ewy1Mu1ITy3uCtvaWArJFGJ5Swdx5D0jxCqTRMb1jgZQo0E
> yr2UHUhbIWKM2MagUKB97oizRiYfOqhqWGG9Vbt/7G9e7UkYlZQJUT0VtB8+NHjY5YdInHiVanK8
> 4mDGTXTobiobyrVcL2o4SEdJsJeVyzvmNljSTLLkO5UI+BQs0tOkY9DFHNEHLWJcAwXStchp1HaU
> KrG8CWg5Hoh/xdyRThQkOvzfBIA=
>
>
