Bug #11238 SELECT ... FOR UPDATE executed as consistent read inside LOCK TABLES
Submitted: 10 Jun 2005 11:45 Modified: 6 Oct 2005 0:15
Reporter: Heikki Tuuri Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S1 (Critical)
Version:4.1, 5.0 OS:Any (All)
Assigned to: Antony Curtis CPU Architecture:Any

[10 Jun 2005 11:45] Heikki Tuuri
Description:
Hi!

Vadim found this bug last week. Inside LOCK TABLES, InnoDB executes:

SELECT ... FOR UPDATE;

and

SELECT ... LOCK IN SHARE MODE;

as non-locking consistent reads. This can break application logic, since a consistent read may return stale data.

The bug is also present in 5.0 stored procedures, even without using LOCK TABLES.

Regards,

Heikki

How to repeat:
See above.

Suggested fix:
MySQL should store to thd->lex information about a lock clause in a SELECT. Then the code in ha_innobase::start_stmt() could check it. The code currently looks at thd->lex->lock_option, but that does NOT contain the required information.

Regards,

Heikki
[20 Jul 2005 23:16] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/internals/27395
[20 Jul 2005 23:18] Antony Curtis
Heikki,
 Is it possible to provide a simple test case or test the cset attached to this case?
Antony,
[21 Jul 2005 10:23] Heikki Tuuri
Antony,

the patch just contains a new argument to the ::start_stmt() function. Where are the rest of changes?

The knowledge about a ... FOR UPDATE or ... LOCK IN SHARE MODE clause must be gathered in the .yy code. From there it must be passed to the handlers.

Since it is a syntactic thing, I think you should also add the information to the lex struct, like I suggested.

Regards,

Heikki
[21 Jul 2005 10:53] Antony Curtis
Heikki,

I don't believe any further changes are required in the parser - data is stored when the user does a ... FOR UPDATE or ... LOCK IN SHARE MODE, but the infomation was only used to check if the locking was compatible and then was discarded. With the changes in the cset, the when a ...FOR UPDATE is parsed, ::start_stmt should expect to see TL_WRITE as the lock type, and when a .. LOCK IN SHARE MODE is parsed, ::start_stmt should expect to see a TL_READ_WITH_SHARED_LOCKS as the provided lock_type... otherwise only should see a TL_READ when performing a SELECT statement.
However, I would like a testcase I can run which demonstrates failure without this cset, so that I may ensure that the code is working as I expect.
Regards,

Antony.
[21 Jul 2005 12:31] Heikki Tuuri
Antony,

you are right.

(gdb) bt
#0  ha_innobase::start_stmt(THD*) (this=0x8c43fe0, thd=0x8c42a40)
    at ha_innodb.cc:5843
#1  0x081d2c85 in check_lock_and_start_stmt (thd=0x8c42a40, table=0x8c4d020,
    lock_type=TL_READ_WITH_SHARED_LOCKS) at sql_base.cc:1964
#2  0x081d36cd in lock_tables(THD*, st_table_list*, unsigned) (thd=0x8c42a40,
    tables=0x8c6b868, count=1) at sql_base.cc:2274
#3  0x081d3020 in open_and_lock_tables(THD*, st_table_list*) (thd=0x8c42a40,
    tables=0x8c6b868) at sql_base.cc:2088
#4  0x081a56d6 in mysql_execute_command(THD*) (thd=0x8c42a40)
    at sql_parse.cc:2397
#5  0x081ae1eb in mysql_parse(THD*, char*, unsigned) (thd=0x8c42a40,
    inBuf=0x8c6b760 "select * from a lock in share mode", length=34)
    at sql_parse.cc:5381
#6  0x081a39bf in dispatch_command(enum_server_command, THD*, char*, unsigned)
    (command=COM_QUERY, thd=0x8c42a40,
    packet=0x8c63701 "select * from a lock in share mode", packet_length=35)
    at sql_parse.cc:1674
#7  0x081a31c6 in do_command(THD*) (thd=0x8c42a40) at sql_parse.cc:1477
#8  0x081a22cd in handle_one_connection (arg=0x8c42a40) at sql_parse.cc:1126
#9  0x40062f60 in pthread_start_thread () from /lib/i686/libpthread.so.0
#10 0x400630fe in pthread_start_thread_event () from /lib/i686/libpthread.so.0
#11 0x401f5327 in clone () from /lib/i686/libc.so.6
(gdb)

MySQL does have the necessary lock information in check_lock_and_start_stmt().

Please run also a stored procedure inside gdb, and check that lock_type is right.

Please post the test results here. Then you have the my permission to push the patch for InnoDB. For the other code, someone else has to review.

Jan Lindstrom can help you to write a test case. Use two connections: one inserts into a table, another is in the AUTOCOMMIT=0 mode and does LOCK TABLES and various types of SELECT ...

Regards,

Heikki
[22 Aug 2005 12:39] Jan Lindström
Here is one possible test case (somehow I remember that I have already sent this):

create table t1(a int not null, b int, primary key(a)) engine = innodb;
insert into t1 values(1,1),(3,3);
commit;
connect (con1, ...)
connect (con2, ...)
connection con1;
set autocommit=0;
lock tables t1 read;
select * from t1 lock in share mode;
connection con2;
set autocommit=0;
send insert into t1 values(1,1);
connection con1;
sleep 1;
show processlist;
select * from t1 lock in share mode;
commit;
connection con2;
commit;

And something similar for select for update.
[1 Sep 2005 14:55] Sergei Golubchik
Heikki, I don't see a bug here. LOCK TABLES prevents other threads from modifying the tbale in question, so there could be no"stale" data.

Could you elaboraye in what scenario it could be a bug ?
[27 Sep 2005 13:16] Heikki Tuuri
Sergei, all,

the bug is that SELECT FOR UPDATE should lock the selected rows with X-locks. That is very different from doing a consistent read which does not lock anything.

I recall Anthony's patch was correct. Has it been pushed now?

I think an automatic test case is secondary. You can test manually.

Regards,

Heikki
[30 Sep 2005 14:12] Marko Mäkelä
Sergei,
you asked for a scenario. Here is one:
User 2 inserts row but does not commit
User 1 does LOCK TABLE, can do a consistent read (SELECT) but not a locking read (SELECT ... LOCK IN SHARE MODE or SELECT ... FOR UPDATE).
[30 Sep 2005 15:17] Sergei Golubchik
This example does not work:

mysql1> insert t1 values (5);
Query OK, 1 row affected (0.00 sec)

mysql2> lock table t1 write;
--- waits

mysql1> commit;
Query OK, 0 rows affected (0.00 sec)

--- mysql2 now
Query OK, 0 rows affected (7.11 sec)
mysql2> 

So, after LOCK TABLE you cannot see stale data
[30 Sep 2005 16:58] Heikki Tuuri
Sergei,

sorry, I forgot that nowadays LOCK TABLES by default also takes an InnoDB table lock. Then the consistent read cannot see stale data in this case.

But if you set

innodb_table_locks=0

in my.cnf, that is not the case.

Also, someone should test if the bug is present in stored procedures, like I said in the original bug report. That is where Vadim originally found this bug.

Regards,

Heikki
[30 Sep 2005 16:59] Heikki Tuuri
Sergei,

sorry, I forgot that nowadays LOCK TABLES by default also takes an InnoDB table lock. Then the consistent read cannot see stale data in this case.

But if you set

innodb_table_locks=0

in my.cnf, that is not the case.

Also, someone should test if the bug is present in stored procedures, like I said in the original bug report. That is where Vadim originally found this bug.

Regards,

Heikki
[30 Sep 2005 18:18] Sergei Golubchik
Fixed in 5.0.15
[30 Sep 2005 18:23] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/internals/30584
[30 Sep 2005 18:32] Sergei Golubchik
fixed in 5.0.14 too :)
[6 Oct 2005 0:15] Paul DuBois
Noted in 5.0.14 changelog.