MySQL Bugs: #32623: MySQL server crashes on UPDATE with UNIQUE index when built with VC++ 2005

Bug #32623	MySQL server crashes on UPDATE with UNIQUE index when built with VC++ 2005
Submitted:	22 Nov 2007 14:17	Modified:	22 Nov 2007 22:11
Reporter:	Steve Hay	Email Updates:
Status:	Won't fix	Impact on me:	None
Category:	MySQL Server	Severity:	S2 (Serious)
Version:	4.1.22	OS:	Windows (XP SP2)
Assigned to:	Heikki Tuuri	CPU Architecture:	Any
Tags:	Contribution

Description:
An InnoDB table in a database contains a unique index on a pair of columns. When an SQL UPDATE statement is run to alter the value of one of the columns in the unique index in a certain row the server crashes.

We're currently using 4.1.23, downloaded in source form from the commercial licence website (http://mysql.mysql.com) some time ago and built using Visual C++ 2005. However, I see on the public website (http://www.mysql.com) that 4.1.23 has apparently not yet been released, so I downloaded the latest released version of 4.1.x (4.1.22) and tried that but it has the same problem.

I've also tried various earlier versions (4.1.18, 4.1.16, 4.1.15 and 4.1.14) and they all have the same problem.

Note that we're building the server using Visual C++ 2005 (aka VC8). The previous version that we were using was 4.1.14 which we had built with Visual C++ 98 (aka VC6) and that does not crash. Unfortunately, however, we are not able to stick with a version built with VC++ 98 because we are linking the MySQL libraries against other software that has been built with VC++ 2005, so we require a VC++ 2005 build of MySQL to match.

Finally, I've also tried the latest version of 5.0.x (5.0.45), built with VC++ 2005, and that works fine. However, we're unable to upgrade to 5.0.x at the moment.

Would a fix to 4.1.x to make it work with VC++ 2005 be possible?

How to repeat:
Run the following commands in a Command Prompt:

mysql test -u root -e "drop table if exists relation;"

mysql test -u root -e "create table relation (parentid int, childid int, unique (parentid, childid)) engine=innodb;"

mysql test -u root -e "insert into relation values (1, 1);"

mysql test -u root -e "update relation set childid=2 where parentid=1;"

The only workaround that I have at the moment is to remove the unique index on (parentid, childid) in the 'relation' table.

I got this when using debug build:

mysqld-debug.exe!_lseeki64
mysqld-debug.exe!my_seek
mysqld-debug.exe!_my_b_read
mysqld-debug.exe!rr_from_tempfile
mysqld-debug.exe!mysql_update
mysqld-debug.exe!mysql_execute_command
mysqld-debug.exe!mysql_parse
mysqld-debug.exe!dispatch_command
mysqld-debug.exe!do_command
mysqld-debug.exe!handle_one_connection
mysqld-debug.exe!pthread_start
mysqld-debug.exe!_callthreadstart
mysqld-debug.exe!_threadstart

I'm 99% sure this has been fixed in 5.x. Will check.

5.0 should compile ok on 64-bit Windows.

Thank you for the bug report.

c:\dev\4.1>bin\mysql -uroot test
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1 to server version: 4.1.24

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql> drop table if exists relation;
Query OK, 0 rows affected (0.35 sec)

mysql> create table relation (parentid int, childid int, unique (parentid,
    -> childid)) engine=innodb;
Query OK, 0 rows affected (0.22 sec)

mysql> insert into relation values (1, 1);
Query OK, 1 row affected (0.05 sec)

mysql> update relation set childid=2 where parentid=1;
ERROR 2013 (HY000): Lost connection to MySQL server during query

CALL STACK
mysqld.exe!_crt_debugger_hook(int _Reserved=)  Line 65	C
mysqld.exe!_invoke_watson(const wchar_t * pszExpression=0x00000000, const wchar_t * pszFunction=0x00000000, const wchar_t * pszFile=0x00000000, unsigned int nLine=0, unsigned int pReserved=0)  Line 181 + 0x7 bytes	C++
mysqld.exe!_lseeki64(int fh=-1, __int64 pos=6, int mthd=0)  Line 78 + 0x2a bytes	C
mysqld.exe!_my_seek()  + 0x6e bytes	C
mysqld.exe!__my_b_read()  + 0x5a bytes	C
mysqld.exe!ha_innobase::update_row(const unsigned char * old_row=0x04bbef14, unsigned char * new_row=0x993140e3)  Line 2947 + 0x7 bytes	C++
mysqld.exe!mysql_update(THD * thd=0x76509896, st_table_list * table_list=0x04bbf140, List<Item> & fields={...}, List<Item> & values={...}, Item * conds=0x76501ff0, unsigned int order_num=2130567168, st_order * order=0x76501ffe, unsigned long limit=0, enum_duplicates handle_duplicates=DUP_ERROR, int ignore=0)  Line 353 + 0x14 bytes	C++
kernel32.dll!765098ba() 	
[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	
mysqld.exe!sql_alloc(unsigned int Size=8)  Line 42 + 0xd bytes	C++
mysqld.exe!List<create_field>::push_back(create_field * a=0x008e9a20)  Line 380 + 0xa bytes	C++
mysqld.exe!add_field_to_list(THD * thd=0x04bbf520, char * field_name=0x008e0450, enum_field_types type=-1724818713, char * length=0x008e0450, char * decimals=0x008e0490, unsigned int type_modifier=9312313, Item * default_value=0x004e548a, Item * on_update_value=0x00000003, st_lex_string * comment=0x006eda34, char * change=0x008e9868, List<String> * interval_list=0x00000006, charset_info_st * cs=0x00000000, unsigned int uint_geom_type=9345720)  Line 4876	C++
mysqld.exe!yyparse(void * yythd=0x00000000)  Line 12661 + 0xb bytes	C++
mswsock.dll!74882ae0() 	
ntdll.dll!7731a69d() 	
ntdll.dll!7731a6e5() 	
mswsock.dll!74883e9b() 	
mswsock.dll!74883f42() 	
ws2_32.dll!75772ee5() 	
mswsock.dll!74883807() 	
mswsock.dll!74883816() 	
mswsock.dll!74883f42() 	
ws2_32.dll!7577730e() 	
ws2_32.dll!7577358e() 	
ws2_32.dll!75772ee5() 	
mswsock.dll!74883816() 	
ws2_32.dll!75773f03() 	
ws2_32.dll!7577358e() 	
ws2_32.dll!75773f1b() 	
ws2_32.dll!75773f1b() 	
wsock32.dll!7543316a() 	
mysqld.exe!_vio_timeout()  + 0x35 bytes	C
mysqld.exe!do_command(THD * thd=0x0000002f)  Line 1348 + 0x10 bytes	C++
mysqld.exe!handle_one_connection(void * arg=0x008e0450)  Line 1074 + 0xa bytes	C++
mysqld.exe!_win_pthread_setspecific()  + 0x5b bytes	C
mysqld.exe!_callthreadstart()  Line 293 + 0x6 bytes	C
mysqld.exe!_threadstart(void * ptd=0x008c5030)  Line 275 + 0x5 bytes	C
kernel32.dll!765719f1() 	
ntdll.dll!7734d109()

Hmm... maybe the InnoDB part of the stack trace is missing.

ha_innobase::update_row(
/*====================*/
                                        /* out: error number or 0 */
        const mysql_byte*       old_row,/* in: old row in MySQL format */
        mysql_byte*             new_row)/* in: new row in MySQL format */
{
        row_prebuilt_t* prebuilt = (row_prebuilt_t*) innobase_prebuilt;
        upd_t*          uvect;
        int             error = 0;

        DBUG_ENTER("ha_innobase::update_row");

        ut_ad(prebuilt->trx ==
                (trx_t*) current_thd->transaction.all.innobase_tid);

        if (table->timestamp_field_type & TIMESTAMP_AUTO_SET_ON_UPDATE)
                table->timestamp_field->set_time();

        if (last_query_id != user_thd->query_id) {
                prebuilt->sql_stat_start = TRUE;
                last_query_id = user_thd->query_id;

                innobase_release_stat_resources(prebuilt->trx);
        }

        if (prebuilt->upd_node) {
                uvect = prebuilt->upd_node->update;
        } else {
                uvect = row_get_prebuilt_update_vector(prebuilt);
        }

        /* Build an update vector from the modified fields in the rows
        (uses upd_buff of the handle) */

        calc_row_difference(uvect, (mysql_byte*) old_row, new_row, table,
                        upd_buff, (ulint)upd_and_key_val_buff_len,
                        prebuilt, user_thd);

        /* This is not a delete */
        prebuilt->upd_node->is_delete = FALSE;

        assert(prebuilt->template_type == ROW_MYSQL_WHOLE_ROW);

        innodb_srv_conc_enter_innodb(prebuilt->trx);

        error = row_update_for_mysql((byte*) old_row, prebuilt);

        innodb_srv_conc_exit_innodb(prebuilt->trx);

        error = convert_error_code_to_mysql(error, user_thd);

        /* Tell InnoDB server that there might be work for
        utility threads: */

        innobase_active_small();

        DBUG_RETURN(error);
}

I do not see how the code above could invoke a file read operation.

I have a vague memory that something with InnoDB 64-bit file reads was fixed a few years ago.

Please test building MySQL-5.0.

The problem here is actually the invalid parameter handler in der C runtime
(new feature since Visual Studio 2005 ,see
http://msdn2.microsoft.com/EN-US/library/ksazx244(VS.80).aspx for description of the feature)

It does sanity checks for some parameters in a chosen subset of the CRT functions. If parameters aren't sane, the function raises an exception.

For lseek() there is a checks that file descriptor is OK (s. description of lseek in MSDN http://msdn2.microsoft.com/En-US/library/1yee101t(VS.80).aspx ). 
-1 is not ok of course and ,instead of returning -1 and setting errno to EBADF, the lseek() decides to commit suicide and/or inform the developer to check the params he passes to this function.

In MySQL 5.x invalid parameter handler has been switched off via _set_invalid_parameter_handler. See my_init.c and the function my_parameter_handler() with the comment suggesting that somebody run onto lseek problem before.

my_parameter_handler is one way to workaround the problem. Unfortunately it disabled all checks and these are a handy debugging tool sometimes.

Yet another simple workaround is to check fd<0 in my_seek(), set errno=EBADF and return -1 (what a Posix compliant lseek() implementation would do so anyway).

Less simple, yet probably proper way to fix is to understand why my_seek() is actually called on a file that has not been opened and to track seek_not_done flag in the IO cache code.

Miguel or Vladislav,

can you find out what file it is trying to open?

InnoDB should have checks that file opening succeeds.

One possibility is that it is trying to open the temp file that InnoDB uses to buffer error messages.

Regards,

Heikki

Heikki, the problem is actually that there is no open() at all, but 
lseek() on a file that is not opened. I also believe that InnoDB is not involved here at all.

The whole action happens in sql_update.cpp in function mysql_update()

where a temporary IO_CACHE is created (line 241-243)

  IO_CACHE tempfile;
      if (open_cached_file(&tempfile, mysql_tmpdir,TEMP_PREFIX,
			   DISK_BUFFER_SIZE, MYF(MY_WME)))

(tempfile is not backed up by real file after that, so that tempfile.file is -1 after that)

tempfile gets reinited later at mysql_update.cpp,line 301

      if (reinit_io_cache(&tempfile,READ_CACHE,0L,0,0))
	error=1; /* purecov: inspected */
      select->file=tempfile;			// Read row ptrs from this file

and accessed at line mysql_update.cpp, line 324
  while (!(error=info.read_record(&info)) && !thd->killed)

info.read_record is a function pointer set to rr_from_tempfile()

rr_from_tempfile() calls _my_b_read(), passing tempfile variable from mysql_update() above as the first parameter. tempfile is still not backed by an open file and has filedescriptor set to -1 . 

_my_b_read() calls my_seek() with filedescriptor -1.
mf_iocache.c line 442
if (info->seek_not_done)
{					/* File touched, do seek */
  VOID(my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0)));//my_seek(-1,...)will crash
  info->seek_not_done=0;
}

my_seek() is a wrapper around C runtime's _lseeki64(), so _lseeki64 gets filedescriptor -1 and CRT parameter validation code forces the process to crash
(mysqld-debug.exe started with --console would also bring a C runtime assertion popup)

Vladislav,

how would you fix MySQL?

Regards,

Heikki

I'd probably add the parameter validation to my_seek() function. It is a quick fix and is guaranteed to work exactly the same as pre-Visual Studio 2005. 

Proper fix seems to be hard - there is a lot of (void)my_seek(...) or VOID(my_seek(...)) in MySQL. Because the caller does not care about the return code, every such place is actually suspectible for invalid parameters.

Here is the diff.

--- my_seek.c.old	2006-11-03 03:16:34.000000000 +0100
+++ my_seek.c	2007-11-27 21:13:49.072220200 +0100
@@ -29,6 +29,19 @@
 		   whence, MyFlags));
   DBUG_ASSERT(pos != MY_FILEPOS_ERROR);		/* safety check */
 
+#if(defined __WIN__ && defined _MSC_VER && _MSC_VER >=1400)
+  if(fd < 0)
+  {
+    /* 
+      my_seek is sometimes called with -1 and any error is ignored.
+      C runtime  parameter validation logic available since Visual Studio 2005
+      will cause the process to crash, would so we better do sanity checks ere.
+    */
+    errno=my_errno=EBADF;
+    DBUG_PRINT("error",("fd negative"));
+    DBUG_RETURN(MY_FILEPOS_ERROR);
+  }
+#endif
   newpos=lseek(fd, pos, whence);
   if (newpos == (os_off_t) -1)
   {

A workaround which may be acceptable to some people is to upgrade MySQL to 5.0+.