| Bug #33827 | COMMIT AND CHAIN causes serious Valgrind error | ||
|---|---|---|---|
| Submitted: | 11 Jan 2008 20:54 | Modified: | 1 Dec 2008 8:39 |
| Reporter: | Guilhem Bichot | Email Updates: | |
| Status: | Can't repeat | Impact on me: | |
| Category: | MySQL Server: General | Severity: | S2 (Serious) |
| Version: | 5.1-bk, 5.0-bk | OS: | Linux |
| Assigned to: | CPU Architecture: | Any | |
[26 May 2008 15:59]
Georgi Kodinov
Verified as described in 5.0-bk.
I propose the following fix (consistent with how start_transaction_opt is handled now) :
=== modified file 'sql/sql_yacc.yy'
--- sql/sql_yacc.yy 2008-04-10 00:34:38 +0000
+++ sql/sql_yacc.yy 2008-05-26 15:51:22 +0000
@@ -9438,7 +9438,11 @@
opt_chain:
/* empty */ { $$= (YYTHD->variables.completion_type == 1); }
| AND_SYM NO_SYM CHAIN_SYM { $$=0; }
- | AND_SYM CHAIN_SYM { $$=1; }
+ | AND_SYM CHAIN_SYM
+ {
+ $$=1;
+ Lex->start_transaction_opt= 0;
+ }
;
[26 May 2008 16:00]
Georgi Kodinov
The workaround is to use COMMIT/START TRANSACTION instead of COMMIT AND CHAIN
[26 May 2008 16:36]
Guilhem Bichot
Hello Georgi, your fix fixes COMMIT AND CHAIN. But not all other callers of begin_trans() (like rpl_injector.cc and maybe other, a grep would tell), I had put an analysis in this bug report.
[1 Dec 2008 8:39]
Sergei Golubchik
Guilhem said he cannot repeat it anymore
[8 May 2009 16:56]
MySQL Verification Team
I opened bug #44664 with testcases for both these.
Description: Please re-test with the latest 5.1. May affect 5.0 too. Just doing "COMMIT AND CHAIN" on a freshly started server makes Valgrind say that: begin_trans() (called by end_trans()), when it tests lex->start_transaction_opt is testing uninitialized data. This is true: this lex member is initialized only by BEGIN and START TRANSACTION. Thus, all callers of begin_trans() (including indirect callers, like callers of end_trans()) which are not BEGIN or START TRANSACTION, are using it uninitialized (do a grep for begin_trans()). How to repeat: ./mtr --mem --start-and-exit --valgrind and issue "COMMIT AND CHAIN" Suggested fix: At least: *** /tmp/bk_rpl_injector.cc-1.11_ZcYMmY 2007-04-12 16:13:46 +02:00 --- edited/rpl_injector.cc 2008-01-11 21:55:06 +01:00 *************** *** 35,40 **** --- 35,41 ---- m_start_pos.m_file_name= my_strdup(log_info.log_file_name, MYF(0)); m_start_pos.m_file_pos= log_info.pos; + m_thd->lex->start_transaction_opt= 0; /* for begin_trans() */ begin_trans(m_thd); thd->set_current_stmt_binlog_row_based(); *** /tmp/bk_sql_parse.cc-1.690_HHgXzH 2007-12-27 17:49:23 +01:00 --- edited/sql_parse.cc 2008-01-11 21:48:29 +01:00 *************** *** 627,632 **** --- 634,640 ---- xa_state_names[thd->transaction.xid_state.xa_state]); DBUG_RETURN(1); } + thd->lex->start_transaction_opt= 0; /* for begin_trans() */ switch (completion) { case COMMIT: /*