Bug #25888 handler::index_read_idx should call ha_index_end on error
Submitted: 27 Jan 2007 6:27 Modified: 28 Mar 2007 19:02
Reporter: Mark Callaghan Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S2 (Serious)
Version:5.0.36-BK, 5.0.30, 5.1.15-BK, 4.1-BK OS:Linux (Linux)
Assigned to: Sergei Golubchik CPU Architecture:Any
Tags: handler, table

[27 Jan 2007 6:27] Mark Callaghan
Description:
The implementation of handler::index_read_idx does not call ha_index_end on error. This is unfortunate for servers compiled in debug mode with storage engines that do not override the implementation of index_read_idx (it is virtual in handler.h).

The problem is that handler::index_read_idx calls ha_index_init which sets handler::inited to INDEX. When index_read() returns an error, index_read_idx does not call ha_index_end and handler::inited is not set to NONE. When close_thread_table() is called, it has a DBUG_ASSERT that fails because table->file->inited != NONE.

How to repeat:
The code in question:

int handler::index_read_idx(byte * buf, uint index, const byte * key,
			     uint key_len, enum ha_rkey_function find_flag)
{
  int error= ha_index_init(index);
  if (!error)
    error= index_read(buf, key, key_len, find_flag);
  if (!error)
    error= ha_index_end();
  return error;
}

Suggested fix:
Remove this code or document that it doesn't work and isn't used (at least not by MyISAM and InnoDB.
[28 Jan 2007 9:54] Valeriy Kravchuk
Thank you for a bug report. Even in latest 5.0.36-BK one can find the same code starting from line 2711 in sql/handler.cc. Same in 5.1.15-BK (starts at line 3297) and 4.1-BK (starts at line 1701).
[29 Jan 2007 14:39] Mark Leith
This code was actually hit today by Sergei:

http://lists.mysql.com/commits/18937

 int handler::index_read_idx(byte * buf, uint index, const byte * key,
-			     uint key_len, enum ha_rkey_function find_flag)
+                             ulonglong keypart_map,
+                             enum ha_rkey_function find_flag)
 {
-  int error= ha_index_init(index, 0);
-  if (!error)
-    error= index_read(buf, key, key_len, find_flag);
+  int error, error1;
+  error= index_init(index, 0);
   if (!error)
-    error= ha_index_end();
-  return error;
+  {
+    error= index_read(buf, key, keypart_map, find_flag);
+    error1= index_end();
+  }
+  return error ?  error : error1;
 }
[29 Jan 2007 21:23] Mark Leith
Hi Mark,

OK this is done in 5.1, but is not planned to be pushed back in to 5.0 etc. (it's part of the handler reworking within 5.1). 

Is this sufficient for you, or would you prefer this be pushed back to 5.0 as well?

Regards

Mark
[29 Jan 2007 21:26] Mark Callaghan
5.1 only is OK for me.
[28 Mar 2007 19:02] Sergei Golubchik
This was pushed together with WL#3700 patch.
It's in 5.1.17